Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

namesys: properly attach path in name.Resolve #5660

Merged
merged 1 commit into from
Oct 26, 2018
Merged

Conversation

magik6k
Copy link
Member

@magik6k magik6k commented Oct 26, 2018

This got broken in the async namesys refactor, somehow there were no tests to catch this.

@magik6k magik6k requested review from Stebalien and djdv October 26, 2018 16:43
@magik6k magik6k requested a review from Kubuxu as a code owner October 26, 2018 16:43
@ghost ghost assigned magik6k Oct 26, 2018
@ghost ghost added the status/in-progress In progress label Oct 26, 2018
Copy link
Contributor

@djdv djdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor remark, otherwise "works on my machine" seal of approval.

ctx := context.Background()
nds, apis, err := makeAPISwarm(ctx, true, 5)
func appendPath(path coreiface.Path, sub string) coreiface.Path {
p, err := coreiface.ParsePath(path.String() + sub)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it will result in the same behaviour, I wonder if we should make a point to use path.Join in these instances.

License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
@magik6k magik6k added kind/bug A bug in existing code (including security flaws) regression and removed kind/bug A bug in existing code (including security flaws) labels Oct 26, 2018
@Stebalien Stebalien merged commit 96334bb into master Oct 26, 2018
@ghost ghost removed the status/in-progress In progress label Oct 26, 2018
@Stebalien Stebalien deleted the fix/resolve-paths branch October 26, 2018 23:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants