From 4985d0ff44e8f45caf1c77e0551077bfd4a9c893 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 12 May 2025 15:06:11 +0100 Subject: [PATCH 1/4] fix bug when removing all PR reviwers Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 0428d13d6..cc5eefb9e 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -283,8 +283,7 @@ func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, id if err != nil { return err } - if (userIds == nil || len(*userIds) == 0) && - (teamIds == nil || len(*teamIds) == 0) { + if userIds == nil && teamIds == nil { return nil } union := githubv4.Boolean(false) From ce710aa24928db04adb97b6d7b8e47a10092be32 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 12 May 2025 15:15:43 +0100 Subject: [PATCH 2/4] Add test to verify removing all reviewers Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit_test.go | 88 +++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index b09ee104e..ed2231c5d 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -446,6 +446,64 @@ func Test_editRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123\n", }, + { + name: "non-interactive remove all reviewers", + input: &EditOptions{ + SelectorArg: "123", + Finder: shared.NewMockFinder("123", &api.PullRequest{ + URL: "https://github.com/OWNER/REPO/pull/123", + }, ghrepo.New("OWNER", "REPO")), + Interactive: false, + Editable: shared.Editable{ + Title: shared.EditableString{ + Value: "new title", + Edited: true, + }, + Body: shared.EditableString{ + Value: "new body", + Edited: true, + }, + Base: shared.EditableString{ + Value: "base-branch-name", + Edited: true, + }, + Reviewers: shared.EditableSlice{ + Remove: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot", "dependabot"}, + Edited: true, + }, + Assignees: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, + Labels: shared.EditableSlice{ + Add: []string{"feature", "TODO", "bug"}, + Remove: []string{"docs"}, + Edited: true, + }, + Projects: shared.EditableProjects{ + EditableSlice: shared.EditableSlice{ + Add: []string{"Cleanup", "CleanupV2"}, + Remove: []string{"Roadmap", "RoadmapV2"}, + Edited: true, + }, + }, + Milestone: shared.EditableString{ + Value: "GA", + Edited: true, + }, + }, + Fetcher: testFetcher{}, + }, + httpStubs: func(reg *httpmock.Registry) { + mockRepoMetadata(reg, false) + mockPullRequestUpdate(reg) + mockPullRequestReviewersUpdate(reg) + mockPullRequestUpdateLabels(reg) + mockProjectV2ItemUpdate(reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, { name: "interactive", input: &EditOptions{ @@ -487,6 +545,27 @@ func Test_editRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123\n", }, + { + name: "interactive remove all reviewers", + input: &EditOptions{ + SelectorArg: "123", + Finder: shared.NewMockFinder("123", &api.PullRequest{ + URL: "https://github.com/OWNER/REPO/pull/123", + }, ghrepo.New("OWNER", "REPO")), + Interactive: true, + Surveyor: testSurveyor{removeAllReviewers: true}, + Fetcher: testFetcher{}, + EditorRetriever: testEditorRetriever{}, + }, + httpStubs: func(reg *httpmock.Registry) { + mockRepoMetadata(reg, false) + mockPullRequestUpdate(reg) + mockPullRequestReviewersUpdate(reg) + mockPullRequestUpdateLabels(reg) + mockProjectV2ItemUpdate(reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -658,7 +737,8 @@ func mockProjectV2ItemUpdate(reg *httpmock.Registry) { type testFetcher struct{} type testSurveyor struct { - skipReviewers bool + skipReviewers bool + removeAllReviewers bool } type testEditorRetriever struct{} @@ -683,7 +763,11 @@ func (s testSurveyor) EditFields(e *shared.Editable, _ string) error { e.Title.Value = "new title" e.Body.Value = "new body" if !s.skipReviewers { - e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + if s.removeAllReviewers { + e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + } else { + e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + } } e.Assignees.Value = []string{"monalisa", "hubot"} e.Labels.Value = []string{"feature", "TODO", "bug"} From 5dca16fd4114fe79eae276fec9b2c244cd578753 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 13 May 2025 10:13:45 +0100 Subject: [PATCH 3/4] Update test case for removing all reviewers Signed-off-by: Babak K. Shandiz --- pkg/cmd/pr/edit/edit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index ed2231c5d..63e380486 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -764,7 +764,7 @@ func (s testSurveyor) EditFields(e *shared.Editable, _ string) error { e.Body.Value = "new body" if !s.skipReviewers { if s.removeAllReviewers { - e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} + e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external", "dependabot"} } else { e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"} } From 5db8cf7c1db7487038647da2e57a75a460839694 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 13 May 2025 12:38:45 +0100 Subject: [PATCH 4/4] Increase `beforePasswordSendTimeout` to 100 us (#10977) Signed-off-by: Babak K. Shandiz --- internal/prompter/accessible_prompter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/prompter/accessible_prompter_test.go b/internal/prompter/accessible_prompter_test.go index ec19a43e0..2b8104e9a 100644 --- a/internal/prompter/accessible_prompter_test.go +++ b/internal/prompter/accessible_prompter_test.go @@ -34,7 +34,7 @@ import ( // but doesn't mandate that prompts always look exactly the same. func TestAccessiblePrompter(t *testing.T) { - beforePasswordSendTimeout := 20 * time.Microsecond + beforePasswordSendTimeout := 100 * time.Microsecond t.Run("Select", func(t *testing.T) { console := newTestVirtualTerminal(t)