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

Changed CLI to use --types flag, a comma separated list of desired an… #68

Merged
merged 1 commit into from
Sep 6, 2017

Conversation

aaron-prindle
Copy link
Collaborator

No description provided.

@aaron-prindle aaron-prindle force-pushed the type-list branch 4 times, most recently from 006437d to b7f283b Compare August 31, 2017 22:17
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.

You'll need to update the tests to match the new format here. Also, can you update the README? We also might want to think about adding a -h command to give users an easy way of finding the available differs through the tool

@aaron-prindle aaron-prindle force-pushed the type-list branch 2 times, most recently from be604f0 to 3de426f Compare September 1, 2017 17:46
@aaron-prindle
Copy link
Collaborator Author

tests and README updated. updating container-diff help or other messaging to include available analyzers can be done in another PR

@aaron-prindle aaron-prindle force-pushed the type-list branch 7 times, most recently from 6a11153 to 256efba Compare September 5, 2017 17:07
@dlorenc
Copy link
Contributor

dlorenc commented Sep 6, 2017

Looks like we accidentally added a "main" file to the repo that's now conflicting. You'll have to delete.

@aaron-prindle
Copy link
Collaborator Author

deleted 'main'

Copy link
Contributor

@dlorenc dlorenc left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM.

cmd/diff_test.go Outdated
@@ -13,7 +13,8 @@ var diffArgNumTests = []testpair{

func TestDiffArgNum(t *testing.T) {
for _, test := range diffArgNumTests {
valid, err := checkDiffArgNum(test.input)
err := checkDiffArgNum(test.input)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit weird of a construct. Do we need err and valid? Could you do something like:

valid := (checkDiffArgNum == nil) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, changed

@aaron-prindle
Copy link
Collaborator Author

the tests output the err on unexpected errors so storing it separately is needed. changing it back

@aaron-prindle
Copy link
Collaborator Author

i can just use (err == nil) in place of valid

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