From a7fdc37b5d733f690d0fdd111e583d2fa834ca0a Mon Sep 17 00:00:00 2001 From: Kousik Mitra Date: Tue, 2 May 2023 22:41:25 +0530 Subject: [PATCH 1/4] Add fill-first option to create pr This will add an flag --fill-first to pr create sub-command. Which will allow users to create pr with details from first commit. --- pkg/cmd/pr/create/create.go | 16 ++++++++++------ pkg/cmd/pr/create/create_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 83421d88b..33a4f71fe 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -45,6 +45,7 @@ type CreateOptions struct { RepoOverride string Autofill bool + FillFirst bool WebMode bool RecoverFile string @@ -167,8 +168,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return errors.New("`--template` is not supported when using `--body` or `--body-file`") } - if !opts.IO.CanPrompt() && !opts.WebMode && !opts.Autofill && (!opts.TitleProvided || !opts.BodyProvided) { - return cmdutil.FlagErrorf("must provide `--title` and `--body` (or `--fill`) when not running interactively") + if !opts.IO.CanPrompt() && !opts.WebMode && !(opts.Autofill || opts.FillFirst) && (!opts.TitleProvided || !opts.BodyProvided) { + return cmdutil.FlagErrorf("must provide `--title` and `--body` (or `--fill` or `fill-first`) when not running interactively") } if runF != nil { @@ -187,6 +188,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default: current branch)") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Do not prompt for title/body and just use commit info") + fl.BoolVar(&opts.FillFirst, "fill-first", false, "Do not prompt for title/body and just use first commit info") fl.StringSliceVarP(&opts.Reviewers, "reviewer", "r", nil, "Request reviews from people or teams by their `handle`") fl.StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") @@ -196,6 +198,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") fl.StringVarP(&opts.Template, "template", "T", "", "Template `file` to use as starting body text") + cmd.MarkFlagsMutuallyExclusive("fill", "fill-first") + _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "base", "head") _ = cmd.RegisterFlagCompletionFunc("reviewer", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { @@ -392,7 +396,7 @@ func createRun(opts *CreateOptions) (err error) { return } -func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) error { +func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, opts *CreateOptions) error { baseRef := ctx.BaseTrackingBranch headRef := ctx.HeadBranch gitClient := ctx.GitClient @@ -402,7 +406,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) e return err } - if len(commits) == 1 { + if len(commits) == 1 || opts.FillFirst { state.Title = commits[0].Title body, err := gitClient.CommitBody(context.Background(), commits[0].Sha) if err != nil { @@ -485,8 +489,8 @@ func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadata Draft: opts.IsDraft, } - if opts.Autofill || !opts.TitleProvided || !opts.BodyProvided { - err := initDefaultTitleBody(ctx, state) + if opts.Autofill || opts.FillFirst || !opts.TitleProvided || !opts.BodyProvided { + err := initDefaultTitleBody(ctx, state, &opts) if err != nil && opts.Autofill { return nil, fmt.Errorf("could not compute title or body defaults: %w", err) } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 84b5cf6fa..b4282ad17 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -168,6 +168,32 @@ func TestNewCmdCreate(t *testing.T) { cli: "-t mytitle --template bug_fix.md --body-file body_file.md", wantsErr: true, }, + { + name: "with fill-first option", + tty: false, + cli: "--fill-first", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + TitleProvided: false, + Body: "", + BodyProvided: false, + Autofill: false, + FillFirst: true, + RecoverFile: "", + WebMode: false, + IsDraft: false, + BaseBranch: "", + HeadBranch: "", + MaintainerCanModify: true, + }, + }, + { + name: "fill and fill-first is mutually exclusive", + tty: false, + cli: "--fill --fill-first", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -210,6 +236,7 @@ func TestNewCmdCreate(t *testing.T) { assert.Equal(t, tt.wantsOpts.Title, opts.Title) assert.Equal(t, tt.wantsOpts.TitleProvided, opts.TitleProvided) assert.Equal(t, tt.wantsOpts.Autofill, opts.Autofill) + assert.Equal(t, tt.wantsOpts.FillFirst, opts.FillFirst) assert.Equal(t, tt.wantsOpts.WebMode, opts.WebMode) assert.Equal(t, tt.wantsOpts.RecoverFile, opts.RecoverFile) assert.Equal(t, tt.wantsOpts.IsDraft, opts.IsDraft) From 1b9906268f9a039bf6265f1e82182ce83d9b738e Mon Sep 17 00:00:00 2001 From: Kousik Mitra Date: Tue, 2 May 2023 23:22:53 +0530 Subject: [PATCH 2/4] Add test for fill first flag --- pkg/cmd/pr/create/create.go | 4 +-- pkg/cmd/pr/create/create_test.go | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 33a4f71fe..a758bf4a4 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -285,7 +285,7 @@ func createRun(opts *CreateOptions) (err error) { ghrepo.FullName(ctx.BaseRepo)) } - if opts.Autofill || (opts.TitleProvided && opts.BodyProvided) { + if opts.Autofill || opts.FillFirst || (opts.TitleProvided && opts.BodyProvided) { err = handlePush(*opts, *ctx) if err != nil { return @@ -491,7 +491,7 @@ func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadata if opts.Autofill || opts.FillFirst || !opts.TitleProvided || !opts.BodyProvided { err := initDefaultTitleBody(ctx, state, &opts) - if err != nil && opts.Autofill { + if err != nil && (opts.Autofill || opts.FillFirst) { return nil, fmt.Errorf("could not compute title or body defaults: %w", err) } } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index b4282ad17..33cb3f19b 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -995,6 +995,49 @@ func Test_createRun(t *testing.T) { }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", }, + { + name: "fill-first flag provided", + tty: true, + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.TitleProvided = false + opts.FillFirst = true + opts.HeadBranch = "feature" + return func() {} + }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register( + "git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature", + 0, + `56b6f8bb7c9e3a30093cd17e48934ce354148e80,first commit of pr + 343jdfe47c9e3a30093cd17e48934ce354148e80,second commit of pr + `, + ) + cs.Register( + "git -c log.ShowSignature=false show -s --pretty=format:%b 56b6f8bb7c9e3a30093cd17e48934ce354148e80", + 0, + "first commit description", + ) + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { + "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } + } + `, + func(input map[string]interface{}) { + assert.Equal(t, "first commit of pr", input["title"], "pr title should be first commit message") + assert.Equal(t, "first commit description", input["body"], "pr body should be first commit description") + }, + ), + ) + }, + expectedOut: "https://github.com/OWNER/REPO/pull/12\n", + expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 38e6287fe8501de40dcc01098858443fa20cc15a Mon Sep 17 00:00:00 2001 From: Kousik Mitra Date: Tue, 2 May 2023 23:42:17 +0530 Subject: [PATCH 3/4] Fix commit order --- pkg/cmd/pr/create/create.go | 12 ++++++++++-- pkg/cmd/pr/create/create_test.go | 7 +++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index a758bf4a4..218be6623 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -405,8 +405,16 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, o if err != nil { return err } - - if len(commits) == 1 || opts.FillFirst { + if opts.FillFirst { + firstCommitIndex := len(commits) - 1 + state.Title = commits[firstCommitIndex].Title + body, err := gitClient.CommitBody(context.Background(), commits[firstCommitIndex].Sha) + if err != nil { + return err + } + state.Body = body + fmt.Print(state.Title, state.Body) + } else if len(commits) == 1 { state.Title = commits[0].Title body, err := gitClient.CommitBody(context.Background(), commits[0].Sha) if err != nil { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 33cb3f19b..9c141ded6 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -1008,12 +1008,11 @@ func Test_createRun(t *testing.T) { cs.Register( "git -c log.ShowSignature=false log --pretty=format:%H,%s --cherry origin/master...feature", 0, - `56b6f8bb7c9e3a30093cd17e48934ce354148e80,first commit of pr - 343jdfe47c9e3a30093cd17e48934ce354148e80,second commit of pr - `, + "56b6f8bb7c9e3a30093cd17e48934ce354148e80,second commit of pr\n"+ + "343jdfe47c9e3a30093cd17e48934ce354148e80,first commit of pr", ) cs.Register( - "git -c log.ShowSignature=false show -s --pretty=format:%b 56b6f8bb7c9e3a30093cd17e48934ce354148e80", + "git -c log.ShowSignature=false show -s --pretty=format:%b 343jdfe47c9e3a30093cd17e48934ce354148e80", 0, "first commit description", ) From 557127c7ca0ddb3adf2376002817d28418be1c93 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 10 Jul 2023 15:16:51 -0700 Subject: [PATCH 4/4] remove stray print --- pkg/cmd/pr/create/create.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 218be6623..6ddbf7c35 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -413,7 +413,6 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, o return err } state.Body = body - fmt.Print(state.Title, state.Body) } else if len(commits) == 1 { state.Title = commits[0].Title body, err := gitClient.CommitBody(context.Background(), commits[0].Sha)