feat(merge): add owner/repo in prompt

Various places during the `gh pr merge` flow show the PR number and
title. Those places are updated to also show the owner/repo.

E.g.
Before:
  Pull request #123 (title) is ready to be merged
After:
  Pull request owner/repo#123 (title) is ready to be merged

There are other places, where only the number is displayed. Those were
intentionally left as is. It made sense to show the owner/repo only when
the extra context of the title was present.

It also fixes the related tests.

cc #8777
This commit is contained in:
nobe4 2024-03-01 13:26:20 +01:00
parent 590208f5d6
commit db93444ba9
No known key found for this signature in database
GPG key ID: 911FE2A36C3403DD
2 changed files with 27 additions and 27 deletions

View file

@ -235,7 +235,7 @@ func (m *mergeContext) warnIfDiverged() {
return
}
_ = m.warnf("%s Pull request #%d (%s) has diverged from local branch\n", m.cs.Yellow("!"), m.pr.Number, m.pr.Title)
_ = m.warnf("%s Pull request %s/%s#%d (%s) has diverged from local branch\n", m.cs.Yellow("!"), m.baseRepo.RepoOwner(), m.baseRepo.RepoName(), m.pr.Number, m.pr.Title)
}
// Check if the current state of the pull request allows for merging
@ -302,7 +302,7 @@ func (m *mergeContext) merge() error {
return cmdutil.FlagErrorf("--merge, --rebase, or --squash required when not running interactively")
}
_ = m.infof("Merging pull request #%d (%s)\n", m.pr.Number, m.pr.Title)
_ = m.infof("Merging pull request %s/%s#%d (%s)\n", m.baseRepo.RepoOwner(), m.baseRepo.RepoName(), m.pr.Number, m.pr.Title)
apiClient := api.NewClientFromHTTP(m.httpClient)
r, err := api.GitHubRepo(apiClient, m.baseRepo)
@ -366,7 +366,7 @@ func (m *mergeContext) merge() error {
case PullRequestMergeMethodSquash:
action = "Squashed and merged"
}
return m.infof("%s %s pull request #%d (%s)\n", m.cs.SuccessIconWithColor(m.cs.Magenta), action, m.pr.Number, m.pr.Title)
return m.infof("%s %s pull request %s/%s#%d (%s)\n", m.cs.SuccessIconWithColor(m.cs.Magenta), action, m.baseRepo.RepoOwner(), m.baseRepo.RepoName(), m.pr.Number, m.pr.Title)
}
// Delete local branch if requested and if allowed.

View file

@ -337,7 +337,7 @@ func TestPrMerge(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`)
r := regexp.MustCompile(`Merged pull request OWNER/REPO#1 \(The title of the PR\)`)
if !r.MatchString(output.Stderr()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
@ -518,7 +518,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`)
r := regexp.MustCompile(`Merged pull request OWNER/REPO#1 \(The title of the PR\)`)
if !r.MatchString(output.Stderr()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
@ -559,7 +559,7 @@ func TestPrMerge_withMatchCommitHeadFlag(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`)
r := regexp.MustCompile(`Merged pull request OWNER/REPO#1 \(The title of the PR\)`)
if !r.MatchString(output.Stderr()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
@ -601,7 +601,7 @@ func TestPrMerge_withAuthorFlag(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Merged pull request #1 \(The title of the PR\)`)
r := regexp.MustCompile(`Merged pull request OWNER/REPO#1 \(The title of the PR\)`)
if !r.MatchString(output.Stderr()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
@ -653,7 +653,7 @@ func TestPrMerge_deleteBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
Deleted local branch blueberries and switched to branch main
Deleted remote branch blueberries
`), output.Stderr())
@ -704,7 +704,7 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
Deleted local branch blueberries and switched to branch fruit
Deleted remote branch blueberries
`), output.Stderr())
@ -753,7 +753,7 @@ func TestPrMerge_deleteBranch_onlyLocally(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
Deleted local branch blueberries and switched to branch main
`), output.Stderr())
}
@ -803,7 +803,7 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
Deleted local branch blueberries and switched to branch fruit
Deleted remote branch blueberries
`), output.Stderr())
@ -850,7 +850,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
Deleted local branch blueberries
Deleted remote branch blueberries
`), output.Stderr())
@ -892,7 +892,7 @@ func Test_nonDivergingPullRequest(t *testing.T) {
}
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
`), output.Stderr())
}
@ -932,8 +932,8 @@ func Test_divergingPullRequestWarning(t *testing.T) {
}
assert.Equal(t, heredoc.Doc(`
! Pull request #10 (Blueberries are a good fruit) has diverged from local branch
Merged pull request #10 (Blueberries are a good fruit)
! Pull request OWNER/REPO#10 (Blueberries are a good fruit) has diverged from local branch
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
`), output.Stderr())
}
@ -972,7 +972,7 @@ func Test_pullRequestWithoutCommits(t *testing.T) {
}
assert.Equal(t, heredoc.Doc(`
Merged pull request #10 (Blueberries are a good fruit)
Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
`), output.Stderr())
}
@ -1010,7 +1010,7 @@ func TestPrMerge_rebase(t *testing.T) {
t.Fatalf("error running command `pr merge`: %v", err)
}
r := regexp.MustCompile(`Rebased and merged pull request #2 \(The title of the PR\)`)
r := regexp.MustCompile(`Rebased and merged pull request OWNER/REPO#2 \(The title of the PR\)`)
if !r.MatchString(output.Stderr()) {
t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr())
@ -1053,7 +1053,7 @@ func TestPrMerge_squash(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Squashed and merged pull request #3 (The title of the PR)
Squashed and merged pull request OWNER/REPO#3 (The title of the PR)
`), output.Stderr())
}
@ -1275,7 +1275,7 @@ func TestPRMergeTTY(t *testing.T) {
t.Fatalf("Got unexpected error running `pr merge` %s", err)
}
assert.Equal(t, "Merging pull request #3 (It was the best of times)\n✓ Merged pull request #3 (It was the best of times)\n", output.Stderr())
assert.Equal(t, "Merging pull request OWNER/REPO#3 (It was the best of times)\n✓ Merged pull request OWNER/REPO#3 (It was the best of times)\n", output.Stderr())
}
func TestPRMergeTTY_withDeleteBranch(t *testing.T) {
@ -1346,8 +1346,8 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
Merging pull request #3 (It was the best of times)
Merged pull request #3 (It was the best of times)
Merging pull request OWNER/REPO#3 (It was the best of times)
Merged pull request OWNER/REPO#3 (It was the best of times)
Deleted local branch blueberries and switched to branch main
Deleted remote branch blueberries
`), output.Stderr())
@ -1438,7 +1438,7 @@ func TestPRMergeTTY_squashEditCommitMsgAndSubject(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
assert.Equal(t, "Merging pull request #123 (title)\n✓ Squashed and merged pull request #123 (title)\n", stderr.String())
assert.Equal(t, "Merging pull request OWNER/REPO#123 (title)\n✓ Squashed and merged pull request OWNER/REPO#123 (title)\n", stderr.String())
}
func TestPRMergeEmptyStrategyNonTTY(t *testing.T) {
@ -1517,7 +1517,7 @@ func TestPRTTY_cancelled(t *testing.T) {
t.Fatalf("got error %v", err)
}
assert.Equal(t, "Merging pull request #123 (title)\nCancelled.\n", output.Stderr())
assert.Equal(t, "Merging pull request OWNER/REPO#123 (title)\nCancelled.\n", output.Stderr())
}
func Test_mergeMethodSurvey(t *testing.T) {
@ -1576,7 +1576,7 @@ func TestMergeRun_autoMerge(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
assert.Equal(t, "✓ Pull request #123 will be automatically merged via squash when all requirements are met\n", stderr.String())
assert.Equal(t, "✓ Pull request #123 will be automatically merged via squash when all requirements are met\n", stderr.String())
}
func TestMergeRun_autoMerge_directMerge(t *testing.T) {
@ -1614,7 +1614,7 @@ func TestMergeRun_autoMerge_directMerge(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
assert.Equal(t, "✓ Merged pull request #123 ()\n", stderr.String())
assert.Equal(t, "✓ Merged pull request OWNER/REPO#123 ()\n", stderr.String())
}
func TestMergeRun_disableAutoMerge(t *testing.T) {
@ -1867,7 +1867,7 @@ func TestPrAddToMergeQueueAdmin(t *testing.T) {
}
assert.Equal(t, "", output.String())
assert.Equal(t, "Merging pull request #1 (The title of the PR)\n✓ Merged pull request #1 (The title of the PR)\n", output.Stderr())
assert.Equal(t, "Merging pull request OWNER/REPO#1 (The title of the PR)\n✓ Merged pull request OWNER/REPO#1 (The title of the PR)\n", output.Stderr())
}
func TestPrAddToMergeQueueAdminWithMergeStrategy(t *testing.T) {
@ -1906,7 +1906,7 @@ func TestPrAddToMergeQueueAdminWithMergeStrategy(t *testing.T) {
}
assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Merged pull request #1 (The title of the PR)\n", output.Stderr())
assert.Equal(t, "✓ Merged pull request OWNER/REPO#1 (The title of the PR)\n", output.Stderr())
}
type testEditor struct{}