-
Notifications
You must be signed in to change notification settings - Fork 47
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
Adds a "provider" flag to update a install commands #49
Conversation
This flags allows installing binaries from providers that doesn't follow our initial URL resolution scheme.
SilenceUsage: true, | ||
SilenceErrors: true, | ||
Args: cobra.MaximumNArgs(2), | ||
Args: cobra.MinimumNArgs(1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this change is strictly necessary, as the number of Args
is not the same as the number of flags. However, if it is needed to make something else work, I don't see a problem with keeping it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this change is unrelated to this PR. I just realized that we didn't configure that correctly and I'm fixing that here since bin install
always requires a minimum of a 1 argument to work :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor typo, otherwise looks good!
Co-authored-by: Sune Keller <sune.keller+github@gmail.com>
Thx for reviewing @sirlatrom . I believe it's ready for a second review and merge if approved. 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This flags allows installing binaries from providers that doesn't follow
our initial URL resolution scheme.