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

name cmd: move subcommands to subdirectory #5392

Merged
merged 2 commits into from
Aug 22, 2018
Merged

Conversation

magik6k
Copy link
Member

@magik6k magik6k commented Aug 20, 2018

Just wanted to do this before ipfs name resolve --stream

  • Moves GetApi/GetNode utils to subpkg to avoid duplicating them
  • Port name commands to the new lib
  • Move ipns name commands into a sub-package

@magik6k magik6k requested a review from Kubuxu as a code owner August 20, 2018 20:11
@ghost ghost assigned magik6k Aug 20, 2018
@ghost ghost added the status/in-progress In progress label Aug 20, 2018
@magik6k magik6k force-pushed the refactor/name-cmd-dir branch 2 times, most recently from 2a3008b to 8aa1fa8 Compare August 20, 2018 20:13
@magik6k magik6k requested a review from Stebalien August 20, 2018 20:14
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM with some minor nits.

}
return strings.NewReader(output.Path.String() + "\n"), nil
},
_, err := w.Write([]byte(output.Path.String() + "\n"))
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit but we might as well use fmt.Writeln(w, output.Path.String()).

@@ -71,17 +71,18 @@ var ipnspsStateCmd = &cmds.Command{
state = "disabled"
}

return strings.NewReader(state + "\n"), nil
},
_, err := w.Write([]byte(state + "\n"))
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

@@ -164,7 +160,26 @@ var ipnspsCancelCmd = &cmds.Command{
state = "no subscription"
}

return strings.NewReader(state + "\n"), nil
},
_, err := w.Write([]byte(state + "\n"))
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

License: MIT
Signed-off-by: Łukasz Magiera <[email protected]>
License: MIT
Signed-off-by: Steven Allen <[email protected]>
@Stebalien
Copy link
Member

@magik6k can you check over my latest change? If that looks good to you, I'll merge.

@Stebalien Stebalien added the need/review Needs a review label Aug 22, 2018
@Stebalien
Copy link
Member

Meh. This is actually just ready to go.

@Stebalien Stebalien removed the need/review Needs a review label Aug 22, 2018
@Stebalien Stebalien merged commit 4fd8a6e into master Aug 22, 2018
@ghost ghost removed the status/in-progress In progress label Aug 22, 2018
@Stebalien Stebalien deleted the refactor/name-cmd-dir branch August 24, 2018 00:07
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.

2 participants