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

Cleanup flag parsing. #123

Merged
merged 2 commits into from
Oct 19, 2017
Merged

Cleanup flag parsing. #123

merged 2 commits into from
Oct 19, 2017

Conversation

dlorenc
Copy link
Contributor

@dlorenc dlorenc commented Oct 19, 2017

This also changes the types flag from a single comma separated list to
a repeated flag that fills an array. This changes the name from types
to type, which is a minor breaking change.

Let me know what you think.

cc @aaron-prindle @nkubala

Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

Nice, yeah I like this better than the comma separated list for sure. WDYT about having the flag be --diff-types, or -d, just for clarity? If we document it it doesn't really matter but I think --types is a bit ambiguous

@r2d4
Copy link
Contributor

r2d4 commented Oct 19, 2017

container-diff diff --diff-types ... ? As a separate PR it might be worthwhile to rethink the commands and flags before they grow anymore...

r2d4
r2d4 previously approved these changes Oct 19, 2017
This also changes the types flag from a single comma separated list to
a repeated flag that fills an array. This changes the name from types
to type, which is a minor breaking change.

Let me know what you think.
@dlorenc dlorenc force-pushed the flags branch 2 times, most recently from fbeea14 to 0ce7e17 Compare October 19, 2017 18:31
@dlorenc
Copy link
Contributor Author

dlorenc commented Oct 19, 2017

OK, tests passing now.

@dlorenc dlorenc merged commit 53c6341 into GoogleContainerTools:master Oct 19, 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.

3 participants