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

update to allow removing pending changes #856

Merged
merged 3 commits into from
Aug 1, 2016

Conversation

endophage
Copy link
Contributor

@endophage endophage commented Jul 20, 2016

Needs tests

Partially addresses #596

Adds --reset and --unstage flags to notary status. reset removes all pending changes for the GUN, unstage allows specific changes to be removed based on their # as shown in notary status

Signed-off-by: David Lawrence [email protected] (github: endophage)

@riyazdf
Copy link
Contributor

riyazdf commented Jul 22, 2016

Seems like the reset flag in here should address #866 (at least on the notary side)

}

func (t *tufCommander) AddToCommand(cmd *cobra.Command) {
cmdTUFInit := cmdTUFInitTemplate.ToCommand(t.tufInit)
cmdTUFInit.Flags().StringVar(&t.rootKey, "rootkey", "", "Root key to initialize the repository with")
cmd.AddCommand(cmdTUFInit)

cmd.AddCommand(cmdTUFStatusTemplate.ToCommand(t.tufStatus))
cmdStatus := cmdTUFStatusTemplate.ToCommand(t.tufStatus)
cmdStatus.Flags().IntSliceVarP(&t.changes, "unstage", "u", nil, "Numbers of changes to delete, as show in status list")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/show/shown

@riyazdf
Copy link
Contributor

riyazdf commented Jul 25, 2016

took this out for a spin and the functionality is great! Just a couple of small nits, can't wait for when tests are ready :)

if err != nil {
return err
}
sort.Sort(fileChanges(fileInfos))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we factor out the first few lines of this function and List into a helper for getting back a sorted list of []os.FileInfo?

Copy link
Contributor Author

@endophage endophage Jul 26, 2016

Choose a reason for hiding this comment

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

I'll just make list and the iterator getFileNames return in a deterministic (sorted) order

David Lawrence added 2 commits August 1, 2016 10:36
Signed-off-by: David Lawrence <[email protected]> (github: endophage)
…change can't be applied before aborting

Signed-off-by: David Lawrence <[email protected]> (github: endophage)
@endophage endophage changed the title WIP: update to allow removing pending changes update to allow removing pending changes Aug 1, 2016
@@ -21,6 +21,24 @@ func (cl *memChangelist) Add(c Change) error {
return nil
}

// Remove deletes the changes found at the given indices
func (cl *memChangelist) Remove(idxs []int) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test for this method? I think the cmd functions only use file changelists which is why it isn't covered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Signed-off-by: David Lawrence <[email protected]> (github: endophage)
@riyazdf
Copy link
Contributor

riyazdf commented Aug 1, 2016

LGTM pending CI

if len(cl.List()) == 0 {
cmd.Printf("No unpublished changes for %s\n", gun)
return nil
}

cmd.Printf("Unpublished changes for %s:\n\n", gun)
cmd.Printf("%-10s%-10s%-12s%s\n", "action", "scope", "type", "path")
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Thanks for fixing this formatting! It looks much nicer!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Big fan of tabwriter now that I've discovered it :-)

@cyli
Copy link
Contributor

cyli commented Aug 1, 2016

LGTM pending CI!

@endophage endophage merged commit 49462f6 into notaryproject:master Aug 1, 2016
@endophage endophage deleted the status_management branch August 1, 2016 22:51
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.

5 participants