Skip to content

Commit 188096b

Browse files
authored
do not merge directly if we cannot enable auto-merge (#165)
1 parent 0d3bf54 commit 188096b

File tree

5 files changed

+31
-60
lines changed

5 files changed

+31
-60
lines changed

__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: 3 additions & 9 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' | 'direct-merged' | 'none'> {
2361+
): Promise<'auto-merged' | 'none'> {
23622362
try {
23632363
this.logger.debug('Enable PR auto-merge');
23642364
const prId = await this.queryPullRequestId(pullRequestNumber);
@@ -2381,15 +2381,9 @@ export class GitHub {
23812381
)
23822382
) {
23832383
this.logger.debug(
2384-
'PR can be merged directly, do it instead of via GitHub auto-merge'
2384+
'Auto-merge cannot be enabled - user probably has auto-merge disabled for their repo'
23852385
);
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';
2386+
return 'none';
23932387
} else {
23942388
throw e;
23952389
}

src/manifest.ts

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

1124-
let directlyMerged = false;
11251124
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
11261125
if (autoMerge) {
1127-
const result = await this.github.enablePullRequestAutoMerge(
1126+
await this.github.enablePullRequestAutoMerge(
11281127
newPullRequest.number,
11291128
autoMerge.mergeMethod
11301129
);
1131-
directlyMerged = result === 'direct-merged';
11321130
}
11331131

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

11411137
return newPullRequest;
11421138
}
@@ -1169,22 +1165,18 @@ export class Manifest {
11691165
}
11701166
);
11711167

1172-
let directlyMerged = false;
11731168
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
11741169
if (autoMerge) {
1175-
const result = await this.github.enablePullRequestAutoMerge(
1170+
await this.github.enablePullRequestAutoMerge(
11761171
updatedPullRequest.number,
11771172
autoMerge.mergeMethod
11781173
);
1179-
directlyMerged = result === 'direct-merged';
11801174
}
11811175

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

11891181
return updatedPullRequest;
11901182
}
@@ -1215,22 +1207,18 @@ export class Manifest {
12151207
// TODO: consider leaving the snooze label
12161208
await this.github.removeIssueLabels([SNOOZE_LABEL], snoozed.number);
12171209

1218-
let directlyMerged = false;
12191210
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
12201211
if (autoMerge) {
1221-
const result = await this.github.enablePullRequestAutoMerge(
1212+
await this.github.enablePullRequestAutoMerge(
12221213
updatedPullRequest.number,
12231214
autoMerge.mergeMethod
12241215
);
1225-
directlyMerged = result === 'direct-merged';
12261216
}
12271217

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

12351223
return updatedPullRequest;
12361224
}

test/github.ts

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

1274-
it('merges release PR directly when an auto-merge given but PR in "clean status"', async () => {
1274+
it('does nothing 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,14 +1312,10 @@ describe('GitHub', () => {
13121312
},
13131313
},
13141314
},
1315-
})
1316-
.put('/repos/fake/fake/pulls/123/merge', {
1317-
merge_method: 'rebase',
1318-
})
1319-
.reply(200);
1315+
});
13201316

13211317
const result = await github.enablePullRequestAutoMerge(123, 'rebase');
1322-
expect(result).to.equal('direct-merged');
1318+
expect(result).to.equal('none');
13231319
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
13241320
req.done();
13251321
});
@@ -1366,14 +1362,10 @@ describe('GitHub', () => {
13661362
},
13671363
},
13681364
},
1369-
})
1370-
.put('/repos/fake/fake/pulls/123/merge', {
1371-
merge_method: 'rebase',
1372-
})
1373-
.reply(200);
1365+
});
13741366

13751367
const result = await github.enablePullRequestAutoMerge(123, 'rebase');
1376-
expect(result).to.equal('direct-merged');
1368+
expect(result).to.equal('none');
13771369
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
13781370
req.done();
13791371
});

test/manifest.ts

Lines changed: 6 additions & 9 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('direct-merged');
4998+
.resolves('none');
49994999
const addPullRequestReviewersStub = sandbox
50005000
.stub(github, 'addPullRequestReviewers')
50015001
.resolves();
@@ -5228,8 +5228,7 @@ version = "3.0.0"
52285228

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

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

52355234
it('enables auto-merge when filters are provided (filters: only commit type, match-all)', async () => {
@@ -5246,7 +5245,7 @@ version = "3.0.0"
52465245
});
52475246
const enablePullRequestAutoMergeStub = sandbox
52485247
.stub(github, 'enablePullRequestAutoMerge')
5249-
.resolves('direct-merged');
5248+
.resolves('none');
52505249
const addPullRequestReviewersStub = sandbox
52515250
.stub(github, 'addPullRequestReviewers')
52525251
.resolves();
@@ -5418,8 +5417,7 @@ version = "3.0.0"
54185417
);
54195418

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

54255423
it('enables auto-merge when filters are provided (filters: build-patch-minor version bump, commit filters, match-at-least-one)', async () => {
@@ -5436,7 +5434,7 @@ version = "3.0.0"
54365434
});
54375435
const enablePullRequestAutoMergeStub = sandbox
54385436
.stub(github, 'enablePullRequestAutoMerge')
5439-
.resolves('direct-merged');
5437+
.resolves('none');
54405438
const addPullRequestReviewersStub = sandbox
54415439
.stub(github, 'addPullRequestReviewers')
54425440
.resolves();
@@ -5755,8 +5753,7 @@ version = "3.0.0"
57555753
);
57565754

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

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

0 commit comments

Comments
 (0)