Add CLI prints for successful operations #974
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
riyazdf
force-pushed
the
success-msg-cli
branch
from
3ed2aee
to
277e67a
Compare
cyli
approved these changes
Sep 22, 2016
| if t.deleteRemote { | ||
| rt, err = getTransport(config, gun, admin) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| remoteDeleteInfo = " and remote" |
There was a problem hiding this comment.
👍 Awesome letting them know they got the remote too!
| } | ||
| return cl.Remove(t.deleteIdx) | ||
| if err != nil { |
There was a problem hiding this comment.
Totally non-blocking stylistic nitpick: you could also do:
if err == nil {
cmd.Printf("...")
}
return err
Which saves one return, and may make it clearer we are doing this because we want to log a success message.
riyazdf
force-pushed
the
success-msg-cli
branch
from
277e67a
to
36f3c94
Compare
HuKeping
reviewed
Sep 23, 2016
| // If it was a success, print to terminal and return nil | ||
| if err == nil { | ||
| cmd.Printf("Successfully reset specified changes for repository %s\n", gun) | ||
| return nil |
There was a problem hiding this comment.
I suppose this line could be removed? If so the comment at line 632 should be updated.
There was a problem hiding this comment.
Definitely could, it's surprising that govet/ineffassign don't seem to care about this. I do like how it's explicit but I'll remove this line 👍
Signed-off-by: Riyaz Faizullabhoy <[email protected]>
riyazdf
force-pushed
the
success-msg-cli
branch
from
36f3c94
to
28195ff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add messages for successful CLI operations, such as
publish(and-p),delete, andkey rotateCloses #973
Signed-off-by: Riyaz Faizullabhoy [email protected]