Skip to content

Commit

Permalink
Revert "do not merge directly if we cannot enable auto-merge (#165)" (#…
Browse files Browse the repository at this point in the history 
…168)

This reverts commit 188096b.
  • Loading branch information
@meorphis
meorphis authored Aug 9, 2024
1 parent c91d9bc commit 20b3f4f
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 31 deletions.
4 changes: 2 additions & 2 deletions __snapshots__/github.ts.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions src/github.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2358,7 +2358,7 @@ export class GitHub {
async enablePullRequestAutoMerge(
pullRequestNumber: number,
mergeMethod: MergeMethod
): Promise<'auto-merged' | 'none'> {
): Promise<'auto-merged' | 'direct-merged' | 'none'> {
try {
this.logger.debug('Enable PR auto-merge');
const prId = await this.queryPullRequestId(pullRequestNumber);
Expand All @@ -2381,9 +2381,15 @@ export class GitHub {
)
) {
this.logger.debug(
'Auto-merge cannot be enabled - user probably has auto-merge disabled for their repo'
'PR can be merged directly, do it instead of via GitHub auto-merge'
);
return 'none';
await this.octokit.pulls.merge({
owner: this.repository.owner,
repo: this.repository.repo,
pull_number: pullRequestNumber,
merge_method: mergeMethod,
});
return 'direct-merged';
} else {
throw e;
}
Expand Down
42 changes: 27 additions & 15 deletions src/manifest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1121,18 +1121,22 @@ export class Manifest {
}
);

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
await this.github.enablePullRequestAutoMerge(
const result = await this.github.enablePullRequestAutoMerge(
newPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

this.github.addPullRequestReviewers({
pullRequestNumber: newPullRequest.number,
reviewers: this.reviewers,
});
if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: newPullRequest.number,
reviewers: this.reviewers,
});
}

return newPullRequest;
}
Expand Down Expand Up @@ -1165,18 +1169,22 @@ export class Manifest {
}
);

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
await this.github.enablePullRequestAutoMerge(
const result = await this.github.enablePullRequestAutoMerge(
updatedPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
}

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

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
await this.github.enablePullRequestAutoMerge(
const result = await this.github.enablePullRequestAutoMerge(
updatedPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
}

return updatedPullRequest;
}
Expand Down
18 changes: 13 additions & 5 deletions test/github.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ describe('GitHub', () => {
req.done();
});

it('does nothing when an auto-merge given but PR in "clean status"', async () => {
it('merges release PR directly when an auto-merge given but PR in "clean status"', async () => {
const mutatePullRequestEnableAutoMergeStub = sandbox
.stub(github, <any>'mutatePullRequestEnableAutoMerge') // eslint-disable-line @typescript-eslint/no-explicit-any
.throws(
Expand Down Expand Up @@ -1312,10 +1312,14 @@ describe('GitHub', () => {
},
},
},
});
})
.put('/repos/fake/fake/pulls/123/merge', {
merge_method: 'rebase',
})
.reply(200);

const result = await github.enablePullRequestAutoMerge(123, 'rebase');
expect(result).to.equal('none');
expect(result).to.equal('direct-merged');
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
req.done();
});
Expand Down Expand Up @@ -1362,10 +1366,14 @@ describe('GitHub', () => {
},
},
},
});
})
.put('/repos/fake/fake/pulls/123/merge', {
merge_method: 'rebase',
})
.reply(200);

const result = await github.enablePullRequestAutoMerge(123, 'rebase');
expect(result).to.equal('none');
expect(result).to.equal('direct-merged');
sinon.assert.calledOnce(mutatePullRequestEnableAutoMergeStub);
req.done();
});
Expand Down
15 changes: 9 additions & 6 deletions test/manifest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4995,7 +4995,7 @@ version = "3.0.0"
});
const enablePullRequestAutoMergeStub = sandbox
.stub(github, 'enablePullRequestAutoMerge')
.resolves('none');
.resolves('direct-merged');
const addPullRequestReviewersStub = sandbox
.stub(github, 'addPullRequestReviewers')
.resolves();
Expand Down Expand Up @@ -5228,7 +5228,8 @@ version = "3.0.0"

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

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

it('enables auto-merge when filters are provided (filters: only commit type, match-all)', async () => {
Expand All @@ -5245,7 +5246,7 @@ version = "3.0.0"
});
const enablePullRequestAutoMergeStub = sandbox
.stub(github, 'enablePullRequestAutoMerge')
.resolves('none');
.resolves('direct-merged');
const addPullRequestReviewersStub = sandbox
.stub(github, 'addPullRequestReviewers')
.resolves();
Expand Down Expand Up @@ -5417,7 +5418,8 @@ version = "3.0.0"
);

expect(enablePullRequestAutoMergeStub.callCount).to.equal(3);
expect(addPullRequestReviewersStub.callCount).to.equal(4);
// only called when not auto-merged
expect(addPullRequestReviewersStub.callCount).to.equal(1);
});

it('enables auto-merge when filters are provided (filters: build-patch-minor version bump, commit filters, match-at-least-one)', async () => {
Expand All @@ -5434,7 +5436,7 @@ version = "3.0.0"
});
const enablePullRequestAutoMergeStub = sandbox
.stub(github, 'enablePullRequestAutoMerge')
.resolves('none');
.resolves('direct-merged');
const addPullRequestReviewersStub = sandbox
.stub(github, 'addPullRequestReviewers')
.resolves();
Expand Down Expand Up @@ -5753,7 +5755,8 @@ version = "3.0.0"
);

expect(enablePullRequestAutoMergeStub.callCount).to.equal(4);
expect(addPullRequestReviewersStub.callCount).to.equal(7);
// only called when not auto-merged
expect(addPullRequestReviewersStub.callCount).to.equal(3);
});

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

0 comments on commit 20b3f4f

Please sign in to comment.