Skip to content

Commit 6995839

Browse files
closePR: rename to closePRAsMerged with a celebratory message
Rename `closePR()` to `closePRAsMerged()` and the local variable `closePR` to `upstreamMergeCommit` to clarify that closing a PR in this context is a positive event: the patch series was merged upstream. Update the comment posted on the PR to congratulate the contributor and explain why the GitHub PR shows as "Closed" rather than "Merged" (because the merge happened in the upstream repo, not on GitHub).
1 parent 32e7aab commit 6995839

3 files changed

Lines changed: 16 additions & 8 deletions

File tree

lib/ci-helper.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ export class CIHelper {
647647
}
648648
}
649649

650-
let closePR: string | undefined;
650+
let upstreamMergeCommit: string | undefined;
651651
const prLabelsToAdd: string[] = [];
652652
for (const branch of this.config.repo.trackingBranches) {
653653
const mergeCommit = await this.identifyMergeCommit(branch, tipCommitInGitGit);
@@ -656,7 +656,7 @@ export class CIHelper {
656656
}
657657

658658
if (this.config.repo.closingBranches.includes(branch)) {
659-
closePR = mergeCommit;
659+
upstreamMergeCommit = mergeCommit;
660660
}
661661

662662
if (!prMeta.mergedIntoUpstream) {
@@ -683,7 +683,7 @@ export class CIHelper {
683683
}
684684

685685
let optionsUpdated = false;
686-
if (closePR) {
686+
if (upstreamMergeCommit) {
687687
if (options.openPRs) {
688688
delete options.openPRs[pullRequestURL];
689689
optionsUpdated = true;
@@ -697,7 +697,7 @@ export class CIHelper {
697697
optionsUpdated = true;
698698
}
699699

700-
await this.github.closePR(prKey, closePR);
700+
await this.github.closePRAsMerged(prKey, upstreamMergeCommit);
701701
}
702702

703703
if (notesUpdated) {

lib/github-glue.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ export class GitHubGlue {
289289
return result.data.map((res: { id: number }) => `${res.id}`);
290290
}
291291

292-
public async closePR(pullRequest: pullRequestKeyInfo, viaMergeCommit: string): Promise<number> {
292+
public async closePRAsMerged(pullRequest: pullRequestKeyInfo, mergeCommit: string): Promise<number> {
293293
const prKey = getPullRequestKey(pullRequest);
294294

295295
await this.ensureAuthenticated(prKey.owner);
@@ -298,8 +298,16 @@ export class GitHubGlue {
298298
...prKey,
299299
});
300300

301+
const body =
302+
"Congratulations! 🎉 Your patch series was merged" +
303+
` into upstream via ${mergeCommit}.\n\n` +
304+
"Note: this pull request will show as" +
305+
' "Closed" rather than "Merged" because the' +
306+
" merge happened in the upstream repository," +
307+
" not on GitHub. This is expected —" +
308+
" your contribution has been accepted!";
301309
const result = await this.client.rest.issues.createComment({
302-
body: `Closed via ${viaMergeCommit}.`,
310+
body,
303311
issue_number: prKey.pull_number,
304312
owner: prKey.owner,
305313
repo: prKey.repo,

tests/github-glue.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ test("pull requests", async () => {
9999
});
100100

101101
if (pullRequestURL.length) {
102-
await github.closePR(pullRequestURL, "Not merged");
102+
await github.closePRAsMerged(pullRequestURL, "Not merged");
103103
}
104104

105105
try {
@@ -212,7 +212,7 @@ test("pull requests", async () => {
212212

213213
await github.addPRLabels(prData.html_url, ["bug"]);
214214

215-
const cNumber = await github.closePR(prData.html_url, "Not merged");
215+
const cNumber = await github.closePRAsMerged(prData.html_url, "Not merged");
216216
expect(cNumber).toBeGreaterThan(id);
217217

218218
// delete local and remote branches

0 commit comments

Comments
 (0)