Skip to content

prevent using a file and arguments at the same time#80

Merged
jakecoffman merged 1 commit intomainfrom
jakecoffman/prevent-confusion
Mar 22, 2023
Merged

prevent using a file and arguments at the same time#80
jakecoffman merged 1 commit intomainfrom
jakecoffman/prevent-confusion

Conversation

@jakecoffman
Copy link
Member

It's currently possible to do dependabot update bundler my/repo -f input.yml which is not clear what should happen.

When you run as dependabot update bundler my/repo this is a convenience to get started with Dependabot CLI. Not every option maps to a flag though, so most of the time you end up with an input file dependabot update -f input.yml. Mixing the two is not intended to be a feature.

This PR prevents using the two together. I also cleaned up the usage output and added an example.

@jakecoffman jakecoffman requested a review from a team as a code owner March 22, 2023 14:08
Copy link
Collaborator

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

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

Thanks for this ❤️

@jakecoffman jakecoffman merged commit 00411ee into main Mar 22, 2023
@jakecoffman jakecoffman deleted the jakecoffman/prevent-confusion branch March 22, 2023 14:29

if file != "" {
if len(cmd.Flags().Args()) > 0 {
return errors.New("cannot use file and arguments together")
Copy link
Member

Choose a reason for hiding this comment

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

Since we're using cobra, is there a reason you didn't use the MarkFlagsMutuallyExclusive option?

My guess is that'd also add some useful output in the help command so it wouldn't have to be manually mentioned like you did in line 30...

Docs here: https://github.com/spf13/cobra/blob/main/user_guide.md

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the params are arguments and not flags I think that won't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants