Change bk build pipeline argument to a flag #275
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jradtilbrook
commented
Jun 10, 2024
|
|
||
| cmd := cobra.Command{ | ||
| DisableFlagsInUseLine: true, | ||
| Use: "cancel [number [pipeline]] [flags]", | ||
| Short: "Cancels a build.", | ||
| Use: "cancel <number> [flags]", |
There was a problem hiding this comment.
As this is a destructive operation and could cause issues when builds are cancelled whilst running, I think requiring an actual build number here is a good idea. That will mean the user won't unintentionally cancel a main branch build which could cause issues with deploys or other things.
jradtilbrook
requested review from
mcncl,
lizrabuya and
PriyaSudip
as code owners
jradtilbrook
commented
Jun 11, 2024
Comment on lines
-24
to
+34
| Use: "new [pipeline] [flags]", | ||
| Short: "Creates a new pipeline build", | ||
| Args: cobra.MaximumNArgs(1), | ||
| Use: "new [flags]", | ||
| Short: "Create a new build", | ||
| Args: cobra.NoArgs, | ||
| Long: heredoc.Doc(` | ||
| Creates a new build for the specified pipeline and output the URL to the build. | ||
|
|
||
| The pipeline can be a {pipeline_slug} or in the format {org_slug}/{pipeline_slug}. | ||
| If the pipeline argument is omitted, it will be resolved using the current directory. | ||
| Create a new build on a pipeline. | ||
| The web URL to the build will be printed to stdout. | ||
| `), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| resolvers := resolver.NewAggregateResolver( | ||
| resolver.ResolveFromPositionalArgument(args, 0, f.Config), | ||
| resolver.ResolveFromFlag(pipeline, f.Config), |
There was a problem hiding this comment.
There is no build argument to this command. So I thought making the pipeline name optional instead of an arg would make sense. Any thoughts?
There was a problem hiding this comment.
I am fine with that behaviour.
lizrabuya
approved these changes
Jun 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the positional pipeline argument to a flag instead. This should make more sense to users to allow specifying which pipeline to operate on within the build commands