From b9963809c2e56332b2facd6699d6054ed77ecc21 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 23:25:34 +0100 Subject: [PATCH 01/16] Use signature-stripped tag annotation content Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 1cfc5ba06..132e2c801 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -516,12 +516,32 @@ func createRun(opts *CreateOptions) error { } func gitTagInfo(client *git.Client, tagName string) (string, error) { - cmd, err := client.Command(context.Background(), "tag", "--list", tagName, "--format=%(contents:subject)%0a%0a%(contents:body)") + contentCmd, err := client.Command(context.Background(), "tag", "--list", tagName, "--format=%(contents)") if err != nil { return "", err } - b, err := cmd.Output() - return string(b), err + content, err := contentCmd.Output() + if err != nil { + return "", err + } + + signatureCmd, err := client.Command(context.Background(), "tag", "--list", tagName, "--format=%(contents:signature)") + if err != nil { + return "", err + } + signature, err := signatureCmd.Output() + if err != nil { + return "", err + } + + if len(signature) == 0 { + // The tag annotation content has no trailing signature to strip out, + // so we return the entire content. + return string(content), nil + } + + body, _ := strings.CutSuffix(string(content), "\n"+string(signature)) + return body, nil } func detectPreviousTag(client *git.Client, headRef string) (string, error) { From df3b0627fd697b30bffd1f11ad887b8795f4cf7d Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 23:26:03 +0100 Subject: [PATCH 02/16] Add test for `gitTagInfo` Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create_test.go | 76 +++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index ed7d99c1e..a66bb5899 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -1751,6 +1751,82 @@ func Test_createRun_interactive(t *testing.T) { } } +func Test_gitTagInfo(t *testing.T) { + const tagName = "foo" + const contentCmd = "git tag --list foo --format=%\\(contents\\)" + const signatureCmd = "git tag --list foo --format=%\\(contents:signature\\)" + + tests := []struct { + name string + runStubs func(*run.CommandStubber) + wantErr string + wantResult string + }{ + { + name: "no signature", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent") + cs.Register(signatureCmd, 0, "") + }, + wantResult: "some\nmultiline\ncontent", + }, + { + name: "with signature", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") + cs.Register(signatureCmd, 0, "-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") + }, + wantResult: "some\nmultiline\ncontent", + }, + { + name: "with signature in content but not as true signature", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") + cs.Register(signatureCmd, 0, "") + }, + wantResult: "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----", + }, + { + name: "error getting content", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 1, "some error") + }, + wantErr: fmt.Sprintf("failed to run git: %s exited with status 1", contentCmd), + }, + { + name: "error getting signature", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "whatever") + cs.Register(signatureCmd, 1, "some error") + }, + wantErr: fmt.Sprintf("failed to run git: %s exited with status 1", signatureCmd), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gitClient := &git.Client{GitPath: "some/path/git"} + + rs, teardown := run.Stub() + defer teardown(t) + if tt.runStubs != nil { + tt.runStubs(rs) + } + + result, err := gitTagInfo(gitClient, tagName) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.wantResult, result) + }) + } +} + func boolPtr(b bool) *bool { return &b } From 46814b37598c9a0dc13f4345dc6152684af93172 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 29 Jul 2024 23:26:19 +0100 Subject: [PATCH 03/16] Add example for `--notes-from-tag` Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 132e2c801..412c93328 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -115,6 +115,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Use release notes from a file $ gh release create v1.2.3 -F changelog.md + + Use annotated tag notes + $ gh release create v1.2.3 --notes-from-tag Don't mark the release as latest $ gh release create v1.2.3 --latest=false From 8ea1b8973a63ccd2fd503ff60bae51d5ee17f372 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 30 Jul 2024 00:21:19 +0100 Subject: [PATCH 04/16] Update tests with changes to `gitTagInfo` function Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create_test.go | 127 +++++++++++--------------- 1 file changed, 53 insertions(+), 74 deletions(-) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index a66bb5899..6dd2b4844 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -412,6 +412,14 @@ func Test_NewCmdCreate(t *testing.T) { } func Test_createRun(t *testing.T) { + const contentCmd = `git tag --list .* --format=%\(contents\)` + const signatureCmd = `git tag --list .* --format=%\(contents:signature\)` + + defaultRunStubs := func(rs *run.CommandStubber) { + rs.Register(contentCmd, 0, "") + rs.Register(signatureCmd, 0, "") + } + tests := []struct { name string isTTY bool @@ -432,9 +440,7 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -464,9 +470,7 @@ func Test_createRun(t *testing.T) { Target: "", DiscussionCategory: "General", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -496,9 +500,7 @@ func Test_createRun(t *testing.T) { BodyProvided: true, Target: "main", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -527,9 +529,7 @@ func Test_createRun(t *testing.T) { Draft: true, Target: "", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -558,9 +558,7 @@ func Test_createRun(t *testing.T) { BodyProvided: true, GenerateNotes: false, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -589,9 +587,7 @@ func Test_createRun(t *testing.T) { BodyProvided: true, GenerateNotes: true, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ "url": "https://api.github.com/releases/123", @@ -621,9 +617,7 @@ func Test_createRun(t *testing.T) { GenerateNotes: true, NotesStartTag: "v1.1.0", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.RESTPayload(200, `{ @@ -664,9 +658,7 @@ func Test_createRun(t *testing.T) { GenerateNotes: true, NotesStartTag: "v1.1.0", }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.RESTPayload(200, `{ @@ -715,9 +707,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ @@ -776,9 +766,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ @@ -838,9 +826,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(200, ``)) }, @@ -868,9 +854,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ @@ -905,9 +889,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.StatusStringResponse(201, `{ @@ -943,9 +925,7 @@ func Test_createRun(t *testing.T) { }, Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(200, ``)) }, @@ -974,9 +954,7 @@ func Test_createRun(t *testing.T) { DiscussionCategory: "general", Concurrency: 1, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, + runStubs: defaultRunStubs, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register(httpmock.REST("HEAD", "repos/OWNER/REPO/releases/tags/v1.2.3"), httpmock.StatusStringResponse(404, ``)) reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ @@ -1027,7 +1005,8 @@ func Test_createRun(t *testing.T) { NotesFromTag: true, }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "some tag message") + rs.Register(contentCmd, 0, "some tag message") + rs.Register(signatureCmd, 0, "") }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -1064,7 +1043,8 @@ func Test_createRun(t *testing.T) { NotesFromTag: true, }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "some tag message") + rs.Register(contentCmd, 0, "some tag message") + rs.Register(signatureCmd, 0, "") }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( @@ -1099,10 +1079,8 @@ func Test_createRun(t *testing.T) { Assets: []*shared.AssetForUpload(nil), NotesFromTag: true, }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "") - }, - wantErr: "cannot generate release notes from tag v1.2.3 as it does not exist locally", + runStubs: defaultRunStubs, + wantErr: "cannot generate release notes from tag v1.2.3 as it does not exist locally", }, } for _, tt := range tests { @@ -1149,6 +1127,13 @@ func Test_createRun(t *testing.T) { } func Test_createRun_interactive(t *testing.T) { + const contentCmd = `git tag --list .* --format=%\(contents\)` + const signatureCmd = `git tag --list .* --format=%\(contents:signature\)` + + defaultRunStubs := func(rs *run.CommandStubber) { + rs.Register(contentCmd, 1, "") + } + tests := []struct { name string httpStubs func(*httpmock.Registry) @@ -1185,9 +1170,7 @@ func Test_createRun_interactive(t *testing.T) { return false, nil }) }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") - }, + runStubs: defaultRunStubs, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/tags"), httpmock.StatusStringResponse(200, `[ { "name": "v1.2.3" }, { "name": "v1.2.2" }, { "name": "v1.0.0" }, { "name": "v0.1.2" } @@ -1234,9 +1217,7 @@ func Test_createRun_interactive(t *testing.T) { return false, nil }) }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") - }, + runStubs: defaultRunStubs, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/tags"), httpmock.StatusStringResponse(200, `[ { "name": "v1.2.2" }, { "name": "v1.0.0" }, { "name": "v0.1.2" } @@ -1283,9 +1264,7 @@ func Test_createRun_interactive(t *testing.T) { return false, nil }) }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") - }, + runStubs: defaultRunStubs, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/tags"), httpmock.StatusStringResponse(200, `[ { "name": "v1.2.2" }, { "name": "v1.0.0" }, { "name": "v0.1.2" } @@ -1332,9 +1311,7 @@ func Test_createRun_interactive(t *testing.T) { return false, nil }) }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") - }, + runStubs: defaultRunStubs, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.StatusStringResponse(200, `{ @@ -1381,7 +1358,7 @@ func Test_createRun_interactive(t *testing.T) { }) }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") + defaultRunStubs(rs) rs.Register(`git describe --tags --abbrev=0 HEAD\^`, 0, "v1.2.2\n") rs.Register(`git .+log .+v1\.2\.2\.\.HEAD$`, 0, "commit subject\n\ncommit body\n") }, @@ -1427,7 +1404,8 @@ func Test_createRun_interactive(t *testing.T) { }) }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "hello from annotated tag") + rs.Register(contentCmd, 0, "hello from annotated tag") + rs.Register(signatureCmd, 0, "") rs.Register(`git describe --tags --abbrev=0 v1\.2\.3\^`, 1, "") }, httpStubs: func(reg *httpmock.Registry) { @@ -1456,7 +1434,8 @@ func Test_createRun_interactive(t *testing.T) { TagName: "v1.2.3", }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "tag exists") + rs.Register(contentCmd, 0, "tag exists") + rs.Register(signatureCmd, 0, "") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.GraphQL("RepositoryFindRef"), @@ -1489,7 +1468,8 @@ func Test_createRun_interactive(t *testing.T) { }) }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "tag exists") + rs.Register(contentCmd, 0, "tag exists") + rs.Register(signatureCmd, 0, "") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), @@ -1536,9 +1516,7 @@ func Test_createRun_interactive(t *testing.T) { return false, nil }) }, - runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") - }, + runStubs: defaultRunStubs, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), httpmock.RESTPayload(200, `{ @@ -1591,7 +1569,7 @@ func Test_createRun_interactive(t *testing.T) { }) }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 1, "") + defaultRunStubs(rs) rs.Register(`git .+log .+v1\.1\.0\.\.HEAD$`, 0, "commit subject\n\ncommit body\n") }, httpStubs: func(reg *httpmock.Registry) { @@ -1639,7 +1617,8 @@ func Test_createRun_interactive(t *testing.T) { }) }, runStubs: func(rs *run.CommandStubber) { - rs.Register(`git tag --list`, 0, "tag exists") + rs.Register(contentCmd, 0, "tag exists") + rs.Register(signatureCmd, 0, "") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.GraphQL("RepositoryFindRef"), @@ -1753,8 +1732,8 @@ func Test_createRun_interactive(t *testing.T) { func Test_gitTagInfo(t *testing.T) { const tagName = "foo" - const contentCmd = "git tag --list foo --format=%\\(contents\\)" - const signatureCmd = "git tag --list foo --format=%\\(contents:signature\\)" + const contentCmd = `git tag --list foo --format=%\(contents\)` + const signatureCmd = `git tag --list foo --format=%\(contents:signature\)` tests := []struct { name string From 8356df0188597c9fbfc692e70e79615c596cb102 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:25:20 +0000 Subject: [PATCH 05/16] build(deps): bump github.com/google/go-containerregistry Bumps [github.com/google/go-containerregistry](https://github.com/google/go-containerregistry) from 0.20.1 to 0.20.2. - [Release notes](https://github.com/google/go-containerregistry/releases) - [Changelog](https://github.com/google/go-containerregistry/blob/main/.goreleaser.yml) - [Commits](https://github.com/google/go-containerregistry/compare/v0.20.1...v0.20.2) --- updated-dependencies: - dependency-name: github.com/google/go-containerregistry dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 5 ++--- go.sum | 10 ++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index a607e4adf..96935c68e 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/gabriel-vasile/mimetype v1.4.5 github.com/gdamore/tcell/v2 v2.5.4 github.com/google/go-cmp v0.6.0 - github.com/google/go-containerregistry v0.20.1 + github.com/google/go-containerregistry v0.20.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/gorilla/websocket v1.5.3 github.com/hashicorp/go-multierror v1.1.1 @@ -70,9 +70,8 @@ require ( github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 // indirect github.com/dlclark/regexp2 v1.4.0 // indirect - github.com/docker/cli v24.0.0+incompatible // indirect + github.com/docker/cli v27.1.1+incompatible // indirect github.com/docker/distribution v2.8.2+incompatible // indirect - github.com/docker/docker v24.0.9+incompatible // indirect github.com/docker/docker-credential-helpers v0.7.0 // indirect github.com/fatih/color v1.16.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect diff --git a/go.sum b/go.sum index ffb608ffb..11eeb5411 100644 --- a/go.sum +++ b/go.sum @@ -131,12 +131,10 @@ github.com/distribution/reference v0.5.0 h1:/FUIFXtfc/x2gpa5/VGfiGLuOIdYa1t65IKK github.com/distribution/reference v0.5.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/dlclark/regexp2 v1.4.0 h1:F1rxgk7p4uKjwIQxBs9oAXe5CqrXlCduYEJvrF4u93E= github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= -github.com/docker/cli v24.0.0+incompatible h1:0+1VshNwBQzQAx9lOl+OYCTCEAD8fKs/qeXMx3O0wqM= -github.com/docker/cli v24.0.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v27.1.1+incompatible h1:goaZxOqs4QKxznZjjBWKONQci/MywhtRv2oNn0GkeZE= +github.com/docker/cli v27.1.1+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v2.8.2+incompatible h1:T3de5rq0dB1j30rp0sA2rER+m322EBzniBPB6ZIzuh8= github.com/docker/distribution v2.8.2+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= -github.com/docker/docker v24.0.9+incompatible h1:HPGzNmwfLZWdxHqK9/II92pyi1EpYKsAqcl4G0Of9v0= -github.com/docker/docker v24.0.9+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker-credential-helpers v0.7.0 h1:xtCHsjxogADNZcdv1pKUHXryefjlVRqWqIhk/uXJp0A= github.com/docker/docker-credential-helpers v0.7.0/go.mod h1:rETQfLdHNT3foU5kuNkFR1R1V12OJRRO5lzt2D1b5X0= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= @@ -201,8 +199,8 @@ github.com/google/certificate-transparency-go v1.2.1 h1:4iW/NwzqOqYEEoCBEFP+jPbB github.com/google/certificate-transparency-go v1.2.1/go.mod h1:bvn/ytAccv+I6+DGkqpvSsEdiVGramgaSC6RD3tEmeE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-containerregistry v0.20.1 h1:eTgx9QNYugV4DN5mz4U8hiAGTi1ybXn0TPi4Smd8du0= -github.com/google/go-containerregistry v0.20.1/go.mod h1:YCMFNQeeXeLF+dnhhWkqDItx/JSkH01j1Kis4PsjzFI= +github.com/google/go-containerregistry v0.20.2 h1:B1wPJ1SN/S7pB+ZAimcciVD+r+yV/l/DSArMxlbwseo= +github.com/google/go-containerregistry v0.20.2/go.mod h1:z38EKdKh4h7IP2gSfUUqEvalZBqs6AoLeWfUy34nQC8= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o= From e2275d7ef62c13fe63c80dd204e2a64417be2d8c Mon Sep 17 00:00:00 2001 From: benebsiny Date: Wed, 7 Aug 2024 23:14:15 +0800 Subject: [PATCH 06/16] Add `pr create --editor` --- pkg/cmd/pr/create/create.go | 186 ++++++++++++++++++++----------- pkg/cmd/pr/create/create_test.go | 92 +++++++++++++++ 2 files changed, 211 insertions(+), 67 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 23f302069..acdc0ee82 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -30,15 +30,16 @@ import ( type CreateOptions struct { // This struct stores user input and factory functions - HttpClient func() (*http.Client, error) - GitClient *git.Client - Config func() (gh.Config, error) - IO *iostreams.IOStreams - Remotes func() (ghContext.Remotes, error) - Branch func() (string, error) - Browser browser.Browser - Prompter shared.Prompt - Finder shared.PRFinder + HttpClient func() (*http.Client, error) + GitClient *git.Client + Config func() (gh.Config, error) + IO *iostreams.IOStreams + Remotes func() (ghContext.Remotes, error) + Branch func() (string, error) + Browser browser.Browser + Prompter shared.Prompt + Finder shared.PRFinder + TitledEditSurvey func(string, string) (string, string, error) TitleProvided bool BodyProvided bool @@ -49,6 +50,7 @@ type CreateOptions struct { Autofill bool FillVerbose bool FillFirst bool + EditorMode bool WebMode bool RecoverFile string @@ -88,14 +90,15 @@ type CreateContext struct { func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { opts := &CreateOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - GitClient: f.GitClient, - Config: f.Config, - Remotes: f.Remotes, - Branch: f.Branch, - Browser: f.Browser, - Prompter: f.Prompter, + IO: f.IOStreams, + HttpClient: f.HttpClient, + GitClient: f.GitClient, + Config: f.Config, + Remotes: f.Remotes, + Branch: f.Branch, + Browser: f.Browser, + Prompter: f.Prompter, + TitledEditSurvey: shared.TitledEditSurvey(&shared.UserEditor{Config: f.Config, IO: f.IOStreams}), } var bodyFile string @@ -177,6 +180,20 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("`--fill-verbose` is not supported with `--fill`") } + if err := cmdutil.MutuallyExclusive( + "specify only one of `--editor` or `--web`", + opts.EditorMode, + opts.WebMode, + ); err != nil { + return err + } + + config, err := f.Config() + if err != nil { + return err + } + opts.EditorMode = !opts.WebMode && (opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled") + opts.BodyProvided = cmd.Flags().Changed("body") if bodyFile != "" { b, err := cmdutil.ReadFile(bodyFile, opts.IO.In) @@ -195,6 +212,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("must provide `--title` and `--body` (or `--fill` or `fill-first` or `--fillverbose`) when not running interactively") } + if opts.EditorMode && !opts.IO.CanPrompt() { + return errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") + } + if opts.DryRun && opts.WebMode { return cmdutil.FlagErrorf("`--dry-run` is not supported when using `--web`") } @@ -213,6 +234,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged") fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default [current branch])") + fl.BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the rest text is the body.") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.FillVerbose, "fill-verbose", "", false, "Use commits msg+body for description") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Use commit info for title and body") @@ -315,7 +337,7 @@ func createRun(opts *CreateOptions) (err error) { ghrepo.FullName(ctx.BaseRepo)) } - if opts.FillVerbose || opts.Autofill || opts.FillFirst || (opts.TitleProvided && opts.BodyProvided) { + if !opts.EditorMode && (opts.FillVerbose || opts.Autofill || opts.FillFirst || (opts.TitleProvided && opts.BodyProvided)) { err = handlePush(*opts, *ctx) if err != nil { return @@ -330,71 +352,101 @@ func createRun(opts *CreateOptions) (err error) { } } - if !opts.TitleProvided { - err = shared.TitleSurvey(opts.Prompter, state) - if err != nil { - return - } + action := shared.SubmitAction + if opts.IsDraft { + action = shared.SubmitDraftAction } - defer shared.PreserveInput(opts.IO, state, &err)() + tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) - if !opts.BodyProvided { - templateContent := "" - if opts.RecoverFile == "" { - tpl := shared.NewTemplateManager(client.HTTP(), ctx.BaseRepo, opts.Prompter, opts.RootDirOverride, opts.RepoOverride == "", true) + if opts.EditorMode { + if opts.Template != "" { var template shared.Template - - if opts.Template != "" { - template, err = tpl.Select(opts.Template) - if err != nil { - return - } - } else { - template, err = tpl.Choose() - if err != nil { - return - } + template, err = tpl.Select(opts.Template) + if err != nil { + return } + if state.Title == "" { + state.Title = template.Title() + } + state.Body = string(template.Body()) + } - if template != nil { - templateContent = string(template.Body()) + state.Title, state.Body, err = opts.TitledEditSurvey(state.Title, state.Body) + if err != nil { + return + } + if state.Title == "" { + err = fmt.Errorf("title can't be blank") + return + } + } else { + + if !opts.TitleProvided { + err = shared.TitleSurvey(opts.Prompter, state) + if err != nil { + return } } - err = shared.BodySurvey(opts.Prompter, state, templateContent) - if err != nil { - return + defer shared.PreserveInput(opts.IO, state, &err)() + + if !opts.BodyProvided { + templateContent := "" + if opts.RecoverFile == "" { + var template shared.Template + + if opts.Template != "" { + template, err = tpl.Select(opts.Template) + if err != nil { + return + } + } else { + template, err = tpl.Choose() + if err != nil { + return + } + } + + if template != nil { + templateContent = string(template.Body()) + } + } + + err = shared.BodySurvey(opts.Prompter, state, templateContent) + if err != nil { + return + } } - } - openURL, err = generateCompareURL(*ctx, *state) - if err != nil { - return - } - - allowPreview := !state.HasMetadata() && shared.ValidURL(openURL) && !opts.DryRun - allowMetadata := ctx.BaseRepo.ViewerCanTriage() - action, err := shared.ConfirmPRSubmission(opts.Prompter, allowPreview, allowMetadata, state.Draft) - if err != nil { - return fmt.Errorf("unable to confirm: %w", err) - } - - if action == shared.MetadataAction { - fetcher := &shared.MetadataFetcher{ - IO: opts.IO, - APIClient: client, - Repo: ctx.BaseRepo, - State: state, - } - err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state) + openURL, err = generateCompareURL(*ctx, *state) if err != nil { return } - action, err = shared.ConfirmPRSubmission(opts.Prompter, !state.HasMetadata() && !opts.DryRun, false, state.Draft) + allowPreview := !state.HasMetadata() && shared.ValidURL(openURL) && !opts.DryRun + allowMetadata := ctx.BaseRepo.ViewerCanTriage() + action, err = shared.ConfirmPRSubmission(opts.Prompter, allowPreview, allowMetadata, state.Draft) if err != nil { - return + return fmt.Errorf("unable to confirm: %w", err) + } + + if action == shared.MetadataAction { + fetcher := &shared.MetadataFetcher{ + IO: opts.IO, + APIClient: client, + Repo: ctx.BaseRepo, + State: state, + } + err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.BaseRepo, fetcher, state) + if err != nil { + return + } + + action, err = shared.ConfirmPRSubmission(opts.Prompter, !state.HasMetadata() && !opts.DryRun, false, state.Draft) + if err != nil { + return + } } } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 3b64688c3..e0347945c 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -40,6 +40,7 @@ func TestNewCmdCreate(t *testing.T) { tty bool stdin string cli string + config string wantsErr bool wantsOpts CreateOptions }{ @@ -202,6 +203,64 @@ func TestNewCmdCreate(t *testing.T) { cli: "--web --dry-run", wantsErr: true, }, + { + name: "editor by cli", + tty: true, + cli: "--editor", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + MaintainerCanModify: true, + }, + }, + { + name: "editor by config", + tty: true, + cli: "", + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "", + Body: "", + RecoverFile: "", + WebMode: false, + EditorMode: true, + MaintainerCanModify: true, + }, + }, + { + name: "editor and web", + tty: true, + cli: "--editor --web", + wantsErr: true, + }, + { + name: "can use web even though editor is enabled by config", + tty: true, + cli: `--web --title mytitle --body "issue body"`, + config: "prefer_editor_prompt: enabled", + wantsErr: false, + wantsOpts: CreateOptions{ + Title: "mytitle", + Body: "issue body", + TitleProvided: true, + BodyProvided: true, + RecoverFile: "", + WebMode: true, + EditorMode: false, + MaintainerCanModify: true, + }, + }, + { + name: "editor with non-tty", + tty: false, + cli: "--editor", + wantsErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -215,6 +274,12 @@ func TestNewCmdCreate(t *testing.T) { f := &cmdutil.Factory{ IOStreams: ios, + Config: func() (gh.Config, error) { + if tt.config != "" { + return config.NewFromString(tt.config), nil + } + return config.NewBlankConfig(), nil + }, } var opts *CreateOptions @@ -1372,6 +1437,33 @@ func Test_createRun(t *testing.T) { expectedOut: "https://github.com/OWNER/REPO/pull/12\n", expectedErrOut: "\nCreating pull request for feature into master in OWNER/REPO\n\n", }, + { + name: "editor", + httpStubs: func(r *httpmock.Registry, t *testing.T) { + r.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { + "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } + } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "title", inputs["title"]) + assert.Equal(t, "body", inputs["body"]) + })) + }, + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.EditorMode = true + opts.HeadBranch = "feature" + opts.TitledEditSurvey = func(string, string) (string, string, error) { return "title", "body", nil } + return func() {} + }, + cmdStubs: func(cs *run.CommandStubber) { + cs.Register("git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature", 0, "") + }, + expectedOut: "https://github.com/OWNER/REPO/pull/12\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From c5ea30da4789facca19aaeff6efb2acceb88a57b Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Wed, 7 Aug 2024 10:57:02 -0700 Subject: [PATCH 07/16] Add Acceptance Criteria requirement to triage.md for accepted issues To better facilitate community collaboration and to help reduce the amount of information any collaborator needs to peruse in a given issue, clear Acceptance Criteria on accepted issues should be included. This will reduce the amount of back and forth on PRs submitted by ensuring expectations for a given issue are clear --- docs/triage.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/triage.md b/docs/triage.md index ae9b0c15c..327756c8b 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -8,6 +8,8 @@ triage role. The initial expectation is that the person in the role for the week Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+). +Any issue that is accepted to be done as either an `enhancement`, `bug`, or `docs` requires explicit Acceptance Criteria in a comment on the issue before it is triaged. + To be considered triaged, `enhancement` issues require at least one of the following additional labels: - `core`: reserved for the core CLI team From fa06623f9b428f29aeba54c5f45c005e78334b5a Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Wed, 7 Aug 2024 11:46:22 -0700 Subject: [PATCH 08/16] Update docs/triage.md Co-authored-by: Andy Feller --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 327756c8b..d78a81aed 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -8,7 +8,7 @@ triage role. The initial expectation is that the person in the role for the week Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+). -Any issue that is accepted to be done as either an `enhancement`, `bug`, or `docs` requires explicit Acceptance Criteria in a comment on the issue before it is triaged. +Any issue that is accepted to be done as either an `enhancement`, `bug`, or `docs` requires explicit Acceptance Criteria in a comment on the issue before `needs-triage` label is removed. To be considered triaged, `enhancement` issues require at least one of the following additional labels: From e18c00228314340509ec0c56f15a992a2950ba09 Mon Sep 17 00:00:00 2001 From: benebsiny Date: Thu, 8 Aug 2024 18:07:52 +0800 Subject: [PATCH 09/16] Deduplicate the initialization of editor mode --- pkg/cmd/issue/create/create.go | 15 ++------------- pkg/cmd/pr/create/create.go | 8 ++------ pkg/cmd/pr/shared/survey.go | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 33b583012..1af25180f 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -81,19 +81,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.BaseRepo = f.BaseRepo opts.HasRepoOverride = cmd.Flags().Changed("repo") - if err := cmdutil.MutuallyExclusive( - "specify only one of `--editor` or `--web`", - opts.EditorMode, - opts.WebMode, - ); err != nil { - return err - } - - config, err := f.Config() + var err error + opts.EditorMode, err = prShared.InitEditorMode(f, opts.EditorMode, opts.WebMode, opts.IO.CanPrompt()) if err != nil { return err } - opts.EditorMode = !opts.WebMode && (opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled") titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") @@ -119,9 +111,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co if opts.Interactive && !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("must provide `--title` and `--body` when not running interactively") } - if opts.EditorMode && !opts.IO.CanPrompt() { - return errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") - } if runF != nil { return runF(opts) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index acdc0ee82..d85400202 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -188,11 +188,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return err } - config, err := f.Config() + var err error + opts.EditorMode, err = shared.InitEditorMode(f, opts.EditorMode, opts.WebMode, opts.IO.CanPrompt()) if err != nil { return err } - opts.EditorMode = !opts.WebMode && (opts.EditorMode || config.PreferEditorPrompt("").Value == "enabled") opts.BodyProvided = cmd.Flags().Changed("body") if bodyFile != "" { @@ -212,10 +212,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return cmdutil.FlagErrorf("must provide `--title` and `--body` (or `--fill` or `fill-first` or `--fillverbose`) when not running interactively") } - if opts.EditorMode && !opts.IO.CanPrompt() { - return errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") - } - if opts.DryRun && opts.WebMode { return cmdutil.FlagErrorf("`--dry-run` is not supported when using `--web`") } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 5f4bf7dd9..5b8bde0eb 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -1,6 +1,7 @@ package shared import ( + "errors" "fmt" "strings" @@ -357,3 +358,26 @@ func TitledEditSurvey(editor Editor) func(string, string) (string, string, error return title, strings.TrimSuffix(body, "\n"), nil } } + +func InitEditorMode(f *cmdutil.Factory, editorMode bool, webMode bool, canPrompt bool) (bool, error) { + if err := cmdutil.MutuallyExclusive( + "specify only one of `--editor` or `--web`", + editorMode, + webMode, + ); err != nil { + return false, err + } + + config, err := f.Config() + if err != nil { + return false, err + } + + editorMode = !webMode && (editorMode || config.PreferEditorPrompt("").Value == "enabled") + + if editorMode && !canPrompt { + return false, errors.New("--editor or enabled prefer_editor_prompt configuration are not supported in non-tty mode") + } + + return editorMode, nil +} From 330a385f9e060824e63ad886bd3a97140217e6e6 Mon Sep 17 00:00:00 2001 From: Junichi Sato <22004610+sato11@users.noreply.github.com> Date: Thu, 8 Aug 2024 19:28:03 +0900 Subject: [PATCH 10/16] Document that `gh run download` downloads the latest artifact by default This commit addresses the documentation issue. The discussion at #7018 has confirmed that it is undocumented that the current behavior of `gh run download` with `-n` and no `run-id` downloads the latest artifact. Although the behavior has not been created intentionally, it is the one that should be documented and the future releases should warn before a breaking change. --- pkg/cmd/run/download/download.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/run/download/download.go b/pkg/cmd/run/download/download.go index 18a10df80..99ec45bbe 100644 --- a/pkg/cmd/run/download/download.go +++ b/pkg/cmd/run/download/download.go @@ -42,13 +42,17 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr cmd := &cobra.Command{ Use: "download []", Short: "Download artifacts generated by a workflow run", - Long: heredoc.Doc(` + Long: heredoc.Docf(` Download artifacts generated by a GitHub Actions workflow run. The contents of each artifact will be extracted under separate directories based on the artifact name. If only a single artifact is specified, it will be extracted into the current directory. - `), + + By default, this command downloads the latest artifact created and uploaded through + GitHub Actions. Because workflows can delete or overwrite artifacts, %[1]s%[1]s + must be used to select an artifact from a specific workflow run. + `, "`"), Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` # Download all artifacts generated by a workflow run From ab34d868aa8d99fc33eedcbf5ad64122aa814a34 Mon Sep 17 00:00:00 2001 From: Prabhat Kumar Sahu Date: Fri, 9 Aug 2024 02:14:52 +0530 Subject: [PATCH 11/16] Change `gh repo set-default --view` to print to `stderr` when no default exists (#9431) Change logging of `gh repo set-default --view` to `stderr` when no default exists to better conform with expectations and unix standards. --------- Signed-off-by: Prabhat --- pkg/cmd/repo/setdefault/setdefault.go | 5 ++--- pkg/cmd/repo/setdefault/setdefault_test.go | 13 ++++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/setdefault/setdefault.go b/pkg/cmd/repo/setdefault/setdefault.go index d54d3956a..eb8fcfb5a 100644 --- a/pkg/cmd/repo/setdefault/setdefault.go +++ b/pkg/cmd/repo/setdefault/setdefault.go @@ -121,12 +121,11 @@ func setDefaultRun(opts *SetDefaultOptions) error { if opts.ViewMode { if currentDefaultRepo != nil { fmt.Fprintln(opts.IO.Out, displayRemoteRepoName(currentDefaultRepo)) - } else if opts.IO.IsStdoutTTY() { - fmt.Fprintln(opts.IO.Out, "no default repository has been set; use `gh repo set-default` to select one") + } else { + fmt.Fprintln(opts.IO.ErrOut, "no default repository has been set; use `gh repo set-default` to select one") } return nil } - cs := opts.IO.ColorScheme() if opts.UnsetMode { diff --git a/pkg/cmd/repo/setdefault/setdefault_test.go b/pkg/cmd/repo/setdefault/setdefault_test.go index a89effa14..55a0193d5 100644 --- a/pkg/cmd/repo/setdefault/setdefault_test.go +++ b/pkg/cmd/repo/setdefault/setdefault_test.go @@ -135,6 +135,7 @@ func TestDefaultRun(t *testing.T) { gitStubs func(*run.CommandStubber) prompterStubs func(*prompter.PrompterMock) wantStdout string + wantStderr string wantErr bool errMsg string }{ @@ -175,10 +176,11 @@ func TestDefaultRun(t *testing.T) { Repo: repo1, }, }, - wantStdout: "no default repository has been set; use `gh repo set-default` to select one\n", + wantStderr: "no default repository has been set; use `gh repo set-default` to select one\n", }, { name: "view mode no current default", + tty: false, opts: SetDefaultOptions{ViewMode: true}, remotes: []*context.Remote{ { @@ -186,6 +188,7 @@ func TestDefaultRun(t *testing.T) { Repo: repo1, }, }, + wantStderr: "no default repository has been set; use `gh repo set-default` to select one\n", }, { name: "view mode with base resolved current default", @@ -466,7 +469,7 @@ func TestDefaultRun(t *testing.T) { return &http.Client{Transport: reg}, nil } - io, _, stdout, _ := iostreams.Test() + io, _, stdout, stderr := iostreams.Test() io.SetStdinTTY(tt.tty) io.SetStdoutTTY(tt.tty) io.SetStderrTTY(tt.tty) @@ -498,7 +501,11 @@ func TestDefaultRun(t *testing.T) { return } assert.NoError(t, err) - assert.Equal(t, tt.wantStdout, stdout.String()) + if tt.wantStdout != "" { + assert.Equal(t, tt.wantStdout, stdout.String()) + } else { + assert.Equal(t, tt.wantStderr, stderr.String()) + } }) } } From 18d58b8d84bf1998dd3a0c6a292690069cf9d6b8 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Fri, 9 Aug 2024 13:55:14 -0700 Subject: [PATCH 12/16] Replace `--project.*` flags' `name` with `title` in docs (#9443) With the upcoming migration from v1 project to v2 projects, we'd like the language in our documentation to align with v2 project language. In v2, projects are referred to by `title` and not `name`, though they are functionally equivalent under the hood for the CLI --- pkg/cmd/issue/create/create.go | 2 +- pkg/cmd/issue/edit/edit.go | 4 ++-- pkg/cmd/pr/create/create.go | 2 +- pkg/cmd/pr/edit/edit.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 33b583012..160249e09 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -137,7 +137,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue") cmd.Flags().StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") - cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`") + cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `title`") cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") cmd.Flags().StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") cmd.Flags().StringVarP(&opts.Template, "template", "T", "", "Template `file` to use as starting body text") diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 11a69383d..18067319f 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -153,8 +153,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") - cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the issue to projects by `name`") - cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the issue from projects by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the issue to projects by `title`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the issue from projects by `title`") cmd.Flags().StringVarP(&opts.Editable.Milestone.Value, "milestone", "m", "", "Edit the milestone the issue belongs to by `name`") cmd.Flags().BoolVar(&removeMilestone, "remove-milestone", false, "Remove the milestone association from the issue") diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 23f302069..c6c49ac32 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -220,7 +220,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co 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`") - fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`") + fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `title`") fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") fl.Bool("no-maintainer-edit", false, "Disable maintainer's ability to modify pull request") fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 9dc190011..3c8d73ad3 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -161,8 +161,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself.") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`") cmd.Flags().StringSliceVar(&opts.Editable.Labels.Remove, "remove-label", nil, "Remove labels by `name`") - cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the pull request to projects by `name`") - cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the pull request from projects by `name`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Add, "add-project", nil, "Add the pull request to projects by `title`") + cmd.Flags().StringSliceVar(&opts.Editable.Projects.Remove, "remove-project", nil, "Remove the pull request from projects by `title`") cmd.Flags().StringVarP(&opts.Editable.Milestone.Value, "milestone", "m", "", "Edit the milestone the pull request belongs to by `name`") cmd.Flags().BoolVar(&removeMilestone, "remove-milestone", false, "Remove the milestone association from the pull request") From ac77d6750dcd148c0b58665987132e28ee56aaf0 Mon Sep 17 00:00:00 2001 From: Yukai Chou Date: Sat, 10 Aug 2024 10:20:53 +0800 Subject: [PATCH 13/16] Wrap flags with backticks, continued --- pkg/cmd/repo/view/view.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 136384152..d0370203a 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -45,13 +45,15 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "view []", Short: "View a repository", - Long: `Display the description and the README of a GitHub repository. + Long: heredoc.Docf(` + Display the description and the README of a GitHub repository. -With no argument, the repository for the current directory is displayed. + With no argument, the repository for the current directory is displayed. -With '--web', open the repository in a web browser instead. + With %[1]s--web%[1]s, open the repository in a web browser instead. -With '--branch', view a specific branch of the repository.`, + With %[1]s--branch%[1]s, view a specific branch of the repository. + `, "`"), Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { if len(args) > 0 { From b36b39df60f1d87c26f33a612d5e5881e125c5ca Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sat, 10 Aug 2024 21:29:22 +0100 Subject: [PATCH 14/16] Explain why not looking for signature begin marker Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 412c93328..581255c81 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -528,6 +528,12 @@ func gitTagInfo(client *git.Client, tagName string) (string, error) { return "", err } + // If there is a signature, we should strip it from the end of the content. + // Note that, we can achieve this by looking for markers like "-----BEGIN PGP + // SIGNATURE-----" and cut the remaining text from the content, but this is + // not a safe approach, because, although unlikely, the content can contain + // a signature-like section which we shouldn't leave it as is. So, we need + // to get the tag signature as a whole, if any, and remote it from the content. signatureCmd, err := client.Command(context.Background(), "tag", "--list", tagName, "--format=%(contents:signature)") if err != nil { return "", err From 668dbcefc68bf1b246b75d86b52e75160fc0cd52 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Sat, 10 Aug 2024 21:50:31 +0100 Subject: [PATCH 15/16] Add test cases for PGP, SSH and X.509 signatures Signed-off-by: Babak K. Shandiz --- pkg/cmd/release/create/create_test.go | 34 +++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 6dd2b4844..85c1c3f3f 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -1750,20 +1750,44 @@ func Test_gitTagInfo(t *testing.T) { wantResult: "some\nmultiline\ncontent", }, { - name: "with signature", + name: "with signature (PGP)", runStubs: func(cs *run.CommandStubber) { - cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") - cs.Register(signatureCmd, 0, "-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN PGP SIGNATURE-----\n\nfoo\n-----END PGP SIGNATURE-----") + cs.Register(signatureCmd, 0, "-----BEGIN PGP SIGNATURE-----\n\nfoo\n-----END PGP SIGNATURE-----") + }, + wantResult: "some\nmultiline\ncontent", + }, + { + name: "with signature (PGP, RFC1991)", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN PGP MESSAGE-----\n\nfoo\n-----END PGP MESSAGE-----") + cs.Register(signatureCmd, 0, "-----BEGIN PGP MESSAGE-----\n\nfoo\n-----END PGP MESSAGE-----") + }, + wantResult: "some\nmultiline\ncontent", + }, + { + name: "with signature (SSH)", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SSH SIGNATURE-----\nfoo\n-----END SSH SIGNATURE-----") + cs.Register(signatureCmd, 0, "-----BEGIN SSH SIGNATURE-----\nfoo\n-----END SSH SIGNATURE-----") + }, + wantResult: "some\nmultiline\ncontent", + }, + { + name: "with signature (X.509)", + runStubs: func(cs *run.CommandStubber) { + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SIGNED MESSAGE-----\nfoo\n-----END SIGNED MESSAGE-----") + cs.Register(signatureCmd, 0, "-----BEGIN SIGNED MESSAGE-----\nfoo\n-----END SIGNED MESSAGE-----") }, wantResult: "some\nmultiline\ncontent", }, { name: "with signature in content but not as true signature", runStubs: func(cs *run.CommandStubber) { - cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----") + cs.Register(contentCmd, 0, "some\nmultiline\ncontent\n-----BEGIN PGP SIGNATURE-----\n\nfoo\n-----END PGP SIGNATURE-----") cs.Register(signatureCmd, 0, "") }, - wantResult: "some\nmultiline\ncontent\n-----BEGIN SIGNATURE-----\nfoo\n-----END SIGNATURE-----", + wantResult: "some\nmultiline\ncontent\n-----BEGIN PGP SIGNATURE-----\n\nfoo\n-----END PGP SIGNATURE-----", }, { name: "error getting content", From 369a105c45dc0b99dafca5c39290a1ed2f7bdfc9 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Mon, 12 Aug 2024 10:00:37 -0400 Subject: [PATCH 16/16] Minor grammatical fix --- pkg/cmd/issue/create/create.go | 2 +- pkg/cmd/pr/create/create.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 1af25180f..a5119cb54 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -122,7 +122,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") - cmd.Flags().BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the rest text is the body.") + cmd.Flags().BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the remaining text is the body.") cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue") cmd.Flags().StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`. Use \"@me\" to self-assign.") cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index d85400202..8c91713a1 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -230,7 +230,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)") fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged") fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default [current branch])") - fl.BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the rest text is the body.") + fl.BoolVarP(&opts.EditorMode, "editor", "e", false, "Skip prompts and open the text editor to write the title and body in. The first line is the title and the remaining text is the body.") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.FillVerbose, "fill-verbose", "", false, "Use commits msg+body for description") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Use commit info for title and body")