-
Notifications
You must be signed in to change notification settings - Fork 44
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
Change bk build pipeline argument to a flag #275
Changes from all commits
76a2402
f7b740b
cb40ac3
1ebf1a0
b72d126
220290c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package resolver | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/buildkite/cli/v3/internal/config" | ||
"github.com/buildkite/cli/v3/internal/pipeline" | ||
) | ||
|
||
func ResolveFromFlag(flag string, conf *config.Config) PipelineResolverFn { | ||
return func(context.Context) (*pipeline.Pipeline, error) { | ||
// if the flag is empty, pass through | ||
if flag == "" { | ||
return nil, nil | ||
} | ||
org, name := parsePipelineArg(flag, conf) | ||
|
||
// if we get here, we should be able to parse the value and return an error if not | ||
// this is because a user has explicitly given an input value for us to use - we shoulnt ignore it on error | ||
if org == "" || name == "" { | ||
return nil, fmt.Errorf("unable to parse the input pipeline argument: \"%s\"", flag) | ||
} | ||
|
||
return &pipeline.Pipeline{Name: name, Org: org}, nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,25 +14,24 @@ import ( | |
) | ||
|
||
func NewCmdBuildNew(f *factory.Factory) *cobra.Command { | ||
var message string | ||
var commit string | ||
var branch string | ||
var commit string | ||
var message string | ||
var pipeline string | ||
var web bool | ||
|
||
cmd := cobra.Command{ | ||
DisableFlagsInUseLine: true, | ||
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), | ||
Comment on lines
-24
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine with that behaviour. |
||
resolver.ResolveFromConfig(f.Config, resolver.PickOne), | ||
resolver.ResolveFromRepository(f, resolver.CachedPicker(f.Config, resolver.PickOne)), | ||
) | ||
|
@@ -53,6 +52,9 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command { | |
cmd.Flags().StringVarP(&commit, "commit", "c", "HEAD", "The commit to build.") | ||
cmd.Flags().StringVarP(&branch, "branch", "b", "", "The branch to build. Defaults to the default branch of the pipeline.") | ||
cmd.Flags().BoolVarP(&web, "web", "w", false, "Open the build in a web browser after it has been created.") | ||
cmd.Flags().StringVarP(&pipeline, "pipeline", "p", "", "The pipeline to build. This can be a {pipeline slug} or in the format {org slug}/{pipeline slug}.\n"+ | ||
"If omitted, it will be resolved using the current directory.", | ||
) | ||
cmd.Flags().SortFlags = false | ||
return &cmd | ||
} | ||
|
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.
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.