Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove hardcoded "/" #1984

Merged
merged 3 commits into from
Nov 30, 2015
Merged

Remove hardcoded "/" #1984

merged 3 commits into from
Nov 30, 2015

Conversation

rht
Copy link
Contributor

@rht rht commented Nov 20, 2015

Just small fixes.

Another option, it would be faster to just use strings.Join(elms, "/") and put it in path/path.go, since golang's path does path.Clean of every elements.

@jbenet jbenet added the status/in-progress In progress label Nov 20, 2015
@rht
Copy link
Contributor Author

rht commented Nov 20, 2015

There is also strings.Split -> path.Segments.

@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

(in #1984, stating '/' directly is a crime)

@rht rht changed the title Replace strings.Join(elms, "/") with path.Join(elms...) Replace strings.Join(elms, "/") with path.Join(elms) Nov 24, 2015
@rht rht changed the title Replace strings.Join(elms, "/") with path.Join(elms) Remove hardcoded "/" Nov 24, 2015
License: MIT
Signed-off-by: rht <[email protected]>
@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

Somehow the tests passed.

@rht rht mentioned this pull request Nov 24, 2015
3 tasks
cmd = cmd.Subcommand(name)

if cmd == nil {
pathS := strings.Join(path[0:i], "/")
pathS := path.Join(pth[0:i])
Copy link
Member

Choose a reason for hiding this comment

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

I know you didnt add it, but that [0:i] not being just [:i] is bothering me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 6 occurences of '[0:'. Purging them once for all.

@whyrusleeping
Copy link
Member

LGTM

License: MIT
Signed-off-by: rht <[email protected]>
@jbenet
Copy link
Member

jbenet commented Nov 30, 2015

this LGTM. thanks @rht

jbenet added a commit that referenced this pull request Nov 30, 2015
@jbenet jbenet merged commit 28fa917 into ipfs:dev0.4.0 Nov 30, 2015
@jbenet jbenet removed the status/in-progress In progress label Nov 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants