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

Introduce notary reset command from notary status flags #959

Merged
merged 2 commits into from
Sep 16, 2016

Conversation

riyazdf
Copy link
Contributor

@riyazdf riyazdf commented Sep 14, 2016

Factors out the --reset and --unstage flags from notary status into a new notary reset command.
The notary status command now does not have any flags and cannot mutate any state.

  • notary reset <GUN> --all resets all staged changes for a gun
  • notary reset <GUN> -n 1 -n 2 resets the change items numbered 1 and 2 from notary status <GUN>

Signed-off-by: Riyaz Faizullabhoy [email protected]

@cyli
Copy link
Contributor

cyli commented Sep 14, 2016

Non-blocking nitpick: Wondering if notary reset GUN # # # and notary reset GUN --all would make sense? Alternately, notary reset GUN -n 1 -n 2 to remove specific ones? Just because reset and unstage are sort of similar words to me, it just sounds odd to use one as the flag for the other.

@riyazdf
Copy link
Contributor Author

riyazdf commented Sep 14, 2016

@cyli: agreed! notary reset GUN -n 1 -n 2 and notary reset GUN --all make much more sense in the context of this new command. I'll update this 👍

Copy link
Contributor

@cyli cyli left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for fixing this! LGTM

Signed-off-by: Riyaz Faizullabhoy <[email protected]>
Copy link
Contributor

@endophage endophage left a comment

Choose a reason for hiding this comment

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

LGTM

@endophage endophage merged commit 7ab58e5 into release/0.4.0 Sep 16, 2016
@endophage endophage deleted the notary-reset-cmd branch September 16, 2016 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants