Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.

split container-diff into subcommands: analyze and diff #60

Merged

Conversation

aaron-prindle
Copy link
Collaborator

fixes #56

Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

One comment about putting the flag list together but otherwise lgtm

Use: "analyze",
Short: "Analyzes an image: [image]",
Long: `Analyzes an image using the specifed analyzers as indicated via flags (see documentation for available ones).`,
Run: func(cmd *cobra.Command, args []string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming this is just a copy-paste from the analyze and diff functions? So no need to do a full review of the code?

cmd/analyze.go Outdated

func init() {
RootCmd.AddCommand(analyzeCmd)
analyzeCmd.Flags().BoolVarP(&json, "json", "j", false, "JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).")
Copy link
Contributor

Choose a reason for hiding this comment

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

Since analyze and diff share all the same flags, why not add a helper function here that takes a *cobra.Command and adds the same flag list to both of them?

@aaron-prindle
Copy link
Collaborator Author

added flag sharing fxn

@aaron-prindle
Copy link
Collaborator Author

This PR is to add the subcommands and change the tests, no real logic was changed. The shared logic is in root.go currently and the diff and analyze fxns are in their respective cmd. Will move these out to a /pkg dir and should also probably refactor utils.

@aaron-prindle aaron-prindle merged commit 828be57 into GoogleContainerTools:master Aug 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

container-diff commands should be split into subcommands
2 participants