Skip to content

Commit 20b3f4f

Browse files
authored
Revert "do not merge directly if we cannot enable auto-merge (#165)" (#168)
This reverts commit 188096b.
1 parent c91d9bc commit 20b3f4f

5 files changed

Lines changed: 60 additions & 31 deletions

File tree

__snapshots__/github.ts.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/github.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2358,7 +2358,7 @@ export class GitHub {
23582358
async enablePullRequestAutoMerge(
23592359
pullRequestNumber: number,
23602360
mergeMethod: MergeMethod
2361-
): Promise<'auto-merged' | 'none'> {
2361+
): Promise<'auto-merged' | 'direct-merged' | 'none'> {
23622362
try {
23632363
this.logger.debug('Enable PR auto-merge');
23642364
const prId = await this.queryPullRequestId(pullRequestNumber);
@@ -2381,9 +2381,15 @@ export class GitHub {
23812381
)
23822382
) {
23832383
this.logger.debug(
2384-
'Auto-merge cannot be enabled - user probably has auto-merge disabled for their repo'
2384+
'PR can be merged directly, do it instead of via GitHub auto-merge'
23852385
);
2386-
return 'none';
2386+
await this.octokit.pulls.merge({
2387+
owner: this.repository.owner,
2388+
repo: this.repository.repo,
2389+
pull_number: pullRequestNumber,
2390+
merge_method: mergeMethod,
2391+
});
2392+
return 'direct-merged';
23872393
} else {
23882394
throw e;
23892395
}

src/manifest.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,18 +1121,22 @@ export class Manifest {
11211121
}
11221122
);
11231123

1124+
let directlyMerged = false;
11241125
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
11251126
if (autoMerge) {
1126-
await this.github.enablePullRequestAutoMerge(
1127+
const result = await this.github.enablePullRequestAutoMerge(
11271128
newPullRequest.number,
11281129
autoMerge.mergeMethod
11291130
);
1131+
directlyMerged = result === 'direct-merged';
11301132
}
11311133

1132-
this.github.addPullRequestReviewers({
1133-
pullRequestNumber: newPullRequest.number,
1134-
reviewers: this.reviewers,
1135-
});
1134+
if (!directlyMerged) {
1135+
this.github.addPullRequestReviewers({
1136+
pullRequestNumber: newPullRequest.number,
1137+
reviewers: this.reviewers,
1138+
});
1139+
}
11361140

11371141
return newPullRequest;
11381142
}
@@ -1165,18 +1169,22 @@ export class Manifest {
11651169
}
11661170
);
11671171

1172+
let directlyMerged = false;
11681173
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
11691174
if (autoMerge) {
1170-
await this.github.enablePullRequestAutoMerge(
1175+
const result = await this.github.enablePullRequestAutoMerge(
11711176
updatedPullRequest.number,
11721177
autoMerge.mergeMethod
11731178
);
1179+
directlyMerged = result === 'direct-merged';
11741180
}
11751181

1176-
this.github.addPullRequestReviewers({
1177-
pullRequestNumber: updatedPullRequest.number,
1178-
reviewers: this.reviewers,
1179-
});
1182+
if (!directlyMerged) {
1183+
this.github.addPullRequestReviewers({
1184+
pullRequestNumber: updatedPullRequest.number,
1185+
reviewers: this.reviewers,
1186+
});
1187+
}
11801188

11811189
return updatedPullRequest;
11821190
}
@@ -1207,18 +1215,22 @@ export class Manifest {
12071215
// TODO: consider leaving the snooze label
12081216
await this.github.removeIssueLabels([SNOOZE_LABEL], snoozed.number);
12091217

1218+
let directlyMerged = false;
12101219
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
12111220
if (autoMerge) {
1212-
await this.github.enablePullRequestAutoMerge(
1221+
const result = await this.github.enablePullRequestAutoMerge(
12131222
updatedPullRequest.number,
12141223
autoMerge.mergeMethod
12151224
);
1225+
directlyMerged = result === 'direct-merged';
12161226
}
12171227

1218-
this.github.addPullRequestReviewers({
1219-
pullRequestNumber: updatedPullRequest.number,
1220-
reviewers: this.reviewers,
1221-
});
1228+
if (!directlyMerged) {
1229+
this.github.addPullRequestReviewers({
1230+
pullRequestNumber: updatedPullRequest.number,
1231+
reviewers: this.reviewers,
1232+
});
1233+
}
12221234

12231235
return updatedPullRequest;
12241236
}

test/github.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,7 +1271,7 @@ describe('GitHub', () => {
12711271
req.done();
12721272
});
12731273

1274-
it('does nothing when an auto-merge given but PR in "clean status"', async () => {
1274+
it('merges release PR directly when an auto-merge given but PR in "clean status"', async () => {
12751275
const mutatePullRequestEnableAutoMergeStub = sandbox
12761276
.stub(github, <any>'mutatePullRequestEnableAutoMerge') // eslint-disable-line @typescript-eslint/no-explicit-any
12771277
.throws(
@@ -1312,10 +1312,14 @@ describe('GitHub', () => {
13121312
},
13131313
},
13141314
},
1315-
});
1315+
})
1316+
.put('/repos/fake/fake/pulls/123/merge', {
1317+
merge_method: 'rebase',
1318+
})
1319+
.reply(200);
13161320

13171321
const result = await github.enablePullRequestAutoMerge(123, 'rebase');
1318-
expect(result).to.equal('none');
1322+
expect(result).to.equal('direct-merged');
13191323
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
13201324
req.done();
13211325
});
@@ -1362,10 +1366,14 @@ describe('GitHub', () => {
13621366
},
13631367
},
13641368
},
1365-
});
1369+
})
1370+
.put('/repos/fake/fake/pulls/123/merge', {
1371+
merge_method: 'rebase',
1372+
})
1373+
.reply(200);
13661374

13671375
const result = await github.enablePullRequestAutoMerge(123, 'rebase');
1368-
expect(result).to.equal('none');
1376+
expect(result).to.equal('direct-merged');
13691377
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
13701378
req.done();
13711379
});

test/manifest.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4995,7 +4995,7 @@ version = "3.0.0"
49954995
});
49964996
const enablePullRequestAutoMergeStub = sandbox
49974997
.stub(github, 'enablePullRequestAutoMerge')
4998-
.resolves('none');
4998+
.resolves('direct-merged');
49994999
const addPullRequestReviewersStub = sandbox
50005000
.stub(github, 'addPullRequestReviewers')
50015001
.resolves();
@@ -5228,7 +5228,8 @@ version = "3.0.0"
52285228

52295229
expect(enablePullRequestAutoMergeStub.callCount).to.equal(1);
52305230

5231-
expect(addPullRequestReviewersStub.callCount).to.equal(6);
5231+
// only called when not auto-merged
5232+
expect(addPullRequestReviewersStub.callCount).to.equal(5);
52325233
});
52335234

52345235
it('enables auto-merge when filters are provided (filters: only commit type, match-all)', async () => {
@@ -5245,7 +5246,7 @@ version = "3.0.0"
52455246
});
52465247
const enablePullRequestAutoMergeStub = sandbox
52475248
.stub(github, 'enablePullRequestAutoMerge')
5248-
.resolves('none');
5249+
.resolves('direct-merged');
52495250
const addPullRequestReviewersStub = sandbox
52505251
.stub(github, 'addPullRequestReviewers')
52515252
.resolves();
@@ -5417,7 +5418,8 @@ version = "3.0.0"
54175418
);
54185419

54195420
expect(enablePullRequestAutoMergeStub.callCount).to.equal(3);
5420-
expect(addPullRequestReviewersStub.callCount).to.equal(4);
5421+
// only called when not auto-merged
5422+
expect(addPullRequestReviewersStub.callCount).to.equal(1);
54215423
});
54225424

54235425
it('enables auto-merge when filters are provided (filters: build-patch-minor version bump, commit filters, match-at-least-one)', async () => {
@@ -5434,7 +5436,7 @@ version = "3.0.0"
54345436
});
54355437
const enablePullRequestAutoMergeStub = sandbox
54365438
.stub(github, 'enablePullRequestAutoMerge')
5437-
.resolves('none');
5439+
.resolves('direct-merged');
54385440
const addPullRequestReviewersStub = sandbox
54395441
.stub(github, 'addPullRequestReviewers')
54405442
.resolves();
@@ -5753,7 +5755,8 @@ version = "3.0.0"
57535755
);
57545756

57555757
expect(enablePullRequestAutoMergeStub.callCount).to.equal(4);
5756-
expect(addPullRequestReviewersStub.callCount).to.equal(7);
5758+
// only called when not auto-merged
5759+
expect(addPullRequestReviewersStub.callCount).to.equal(3);
57575760
});
57585761

57595762
it('updates an existing pull request', async () => {

0 commit comments

Comments
 (0)