From c4227455e3ca5aaf288442331ac36c7a6e89f565 Mon Sep 17 00:00:00 2001 From: Daniel Leong Date: Tue, 1 Sep 2020 12:40:36 -0400 Subject: [PATCH 1/4] Prepend PR body defaults to the selected template (if any) See #1570 --- pkg/cmd/pr/shared/title_body_survey.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/pr/shared/title_body_survey.go b/pkg/cmd/pr/shared/title_body_survey.go index 3afeba803..70308e310 100644 --- a/pkg/cmd/pr/shared/title_body_survey.go +++ b/pkg/cmd/pr/shared/title_body_survey.go @@ -154,18 +154,24 @@ func TitleBodySurvey(io *iostreams.IOStreams, editorCommand string, issueState * templateContents := "" if providedBody == "" { + issueState.Body = defs.Body + if len(nonLegacyTemplatePaths) > 0 { var err error templateContents, err = selectTemplate(nonLegacyTemplatePaths, legacyTemplatePath, issueState.Type) if err != nil { return err } - issueState.Body = templateContents + } else if legacyTemplatePath != nil { templateContents = string(githubtemplate.ExtractContents(*legacyTemplatePath)) - issueState.Body = templateContents - } else { - issueState.Body = defs.Body + } + + if templateContents != "" { + if issueState.Body != "" { + issueState.Body += "\n\n" + } + issueState.Body += templateContents } } From a33124414ce972122fc1bc7ddae1c8910c446f64 Mon Sep 17 00:00:00 2001 From: Daniel Leong Date: Tue, 1 Sep 2020 21:29:22 -0400 Subject: [PATCH 2/4] Add a test to verify the behavior of prepending commit body to message --- pkg/cmd/pr/create/create_test.go | 73 ++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 6b79551bf..d6bfb1f76 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "io/ioutil" "net/http" + "os" + "path" "reflect" "strings" "testing" @@ -851,6 +853,77 @@ func TestPRCreate_survey_defaults_monocommit(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } +func TestPRCreate_survey_defaults_monocommit_template(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) + http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` + { "data": { "repository": { "forks": { "nodes": [ + ] } } } } + `)) + http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) + http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(inputs map[string]interface{}) { + eq(t, inputs["repositoryId"], "REPOID") + eq(t, inputs["title"], "the sky above the port") + eq(t, inputs["body"], "was the color of a television\n\n... turned to a dead channel") + eq(t, inputs["baseRefName"], "master") + eq(t, inputs["headRefName"], "feature") + })) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + tmpdir, err := ioutil.TempDir("", "gh-cli") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + templateFp := path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md") + _ = os.MkdirAll(path.Dir(templateFp), 0700) + ioutil.WriteFile(templateFp, []byte("... turned to a dead channel"), 0700) + + cs.Stub("") // git config --get-regexp (determineTrackingBranch) + cs.Stub("") // git show-ref --verify (determineTrackingBranch) + cs.Stub("") // git status + cs.Stub("1234567890,the sky above the port") // git log + cs.Stub("was the color of a television") // git show + cs.Stub(tmpdir) // git rev-parse + cs.Stub("") // git push + + as, surveyTeardown := prompt.InitAskStubber() + defer surveyTeardown() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "title", + Default: true, + }, + { + Name: "body", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "confirmation", + Value: 0, + }, + }) + + output, err := runCommand(http, nil, "feature", true, ``) + eq(t, err, nil) + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") +} + func TestPRCreate_survey_autofill_nontty(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) From 41543b564d1d8a4a9f80fdb8ce9429a38553a960 Mon Sep 17 00:00:00 2001 From: Daniel Leong Date: Tue, 1 Sep 2020 21:37:42 -0400 Subject: [PATCH 3/4] "Check" the error value of ioutil.WriteFile We could, alternatively, use t.Fatal... this matches the MkdirAll invocation from github_template_test, however... --- pkg/cmd/pr/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index d6bfb1f76..b947b22a5 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -889,7 +889,7 @@ func TestPRCreate_survey_defaults_monocommit_template(t *testing.T) { templateFp := path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md") _ = os.MkdirAll(path.Dir(templateFp), 0700) - ioutil.WriteFile(templateFp, []byte("... turned to a dead channel"), 0700) + _ = ioutil.WriteFile(templateFp, []byte("... turned to a dead channel"), 0700) cs.Stub("") // git config --get-regexp (determineTrackingBranch) cs.Stub("") // git show-ref --verify (determineTrackingBranch) From c7e702203bef0db6578cd306067e5f829204347b Mon Sep 17 00:00:00 2001 From: Daniel Leong Date: Tue, 8 Sep 2020 20:25:52 -0400 Subject: [PATCH 4/4] Update template test to reflect inclusion of "default" text Also, avoid excessive amounts of newlines separating the two --- pkg/cmd/pr/create/create_test.go | 2 +- pkg/cmd/pr/shared/title_body_survey.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index b947b22a5..f3426702f 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -316,7 +316,7 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") eq(t, reqBody.Variables.Input.Title, "my title") - eq(t, reqBody.Variables.Input.Body, "Fixes a bug and Closes an issue") + eq(t, reqBody.Variables.Input.Body, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue") eq(t, reqBody.Variables.Input.BaseRefName, "master") eq(t, reqBody.Variables.Input.HeadRefName, "feature") diff --git a/pkg/cmd/pr/shared/title_body_survey.go b/pkg/cmd/pr/shared/title_body_survey.go index 70308e310..83a50e0ff 100644 --- a/pkg/cmd/pr/shared/title_body_survey.go +++ b/pkg/cmd/pr/shared/title_body_survey.go @@ -2,6 +2,7 @@ package shared import ( "fmt" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" @@ -169,6 +170,8 @@ func TitleBodySurvey(io *iostreams.IOStreams, editorCommand string, issueState * if templateContents != "" { if issueState.Body != "" { + // prevent excessive newlines between default body and template + issueState.Body = strings.TrimRight(issueState.Body, "\n") issueState.Body += "\n\n" } issueState.Body += templateContents