Skip to content
This repository was archived by the owner on Sep 13, 2023. It is now read-only.

Commit d72821b

Browse files
authored
Add skipBuildOnDependentsAwaitingMerge config option (#109)
When set, merge commits for PRs will opt out of the destination branch build when there are successful dependent PRs awaiting merge. This prevents multiple destination branch builds from triggering when PRs are landed in quick succession which can affect builds that update src and push back to the repo such as publishing new versions of npm libraries.
1 parent de482f5 commit d72821b

File tree

6 files changed

+64
-9
lines changed

6 files changed

+64
-9
lines changed

config.example.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ module.exports = {
4545
* otherwise return the error message to be displayed on the PR
4646
*/
4747
},
48+
mergeSettings: {
49+
skipBuildOnDependentsAwaitingMerge: false,
50+
},
4851
eventListeners: [
4952
{
5053
event: 'PULL_REQUEST.MERGE.SUCCESS',

src/bitbucket/BitbucketAPI.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import axios, { AxiosResponse } from 'axios';
22
import * as jwtTools from 'atlassian-jwt';
33
import delay from 'delay';
44

5-
import { RepoConfig } from '../types';
5+
import { MergeOptions, RepoConfig } from '../types';
66
import { Logger } from '../lib/Logger';
77
import { bitbucketAuthenticator, axiosPostConfig } from './BitbucketAuthenticator';
88
import { LandRequestStatus } from '../db';
@@ -14,7 +14,7 @@ export class BitbucketAPI {
1414

1515
constructor(private config: RepoConfig) {}
1616

17-
mergePullRequest = async (landRequestStatus: LandRequestStatus) => {
17+
mergePullRequest = async (landRequestStatus: LandRequestStatus, options: MergeOptions = {}) => {
1818
const {
1919
id: landRequestId,
2020
request: {
@@ -23,7 +23,10 @@ export class BitbucketAPI {
2323
},
2424
} = landRequestStatus;
2525
const endpoint = `${this.apiBaseUrl}/pullrequests/${pullRequestId}/merge`;
26-
const message = `pull request #${pullRequestId} merged by Landkid after a successful build rebased on ${targetBranch}`;
26+
let message = `pull request #${pullRequestId} merged by Landkid after a successful build rebased on ${targetBranch}`;
27+
if (options.skipCI) {
28+
message += '\n\n[skip ci]';
29+
}
2730
const requestBody = {
2831
close_source_branch: true,
2932
message,

src/bitbucket/BitbucketClient.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import axios from 'axios';
2-
import { Config } from '../types';
2+
import { Config, MergeOptions } from '../types';
33
import { Logger } from '../lib/Logger';
44
import { BitbucketPipelinesAPI, PipelinesVariables } from './BitbucketPipelinesAPI';
55
import { BitbucketAPI } from './BitbucketAPI';
@@ -98,8 +98,8 @@ export class BitbucketClient {
9898
return this.pipelines.stopLandBuild(buildId, lockId);
9999
}
100100

101-
async mergePullRequest(landRequestStatus: LandRequestStatus) {
102-
return this.bitbucket.mergePullRequest(landRequestStatus);
101+
async mergePullRequest(landRequestStatus: LandRequestStatus, options?: MergeOptions) {
102+
return this.bitbucket.mergePullRequest(landRequestStatus, options);
103103
}
104104

105105
processStatusWebhook(body: any): BB.BuildStatusEvent | null {

src/lib/Runner.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,11 @@ export class Runner {
194194
return true;
195195
};
196196

197-
moveFromAwaitingMerge = async (landRequestStatus: LandRequestStatus, lockId: Date) => {
197+
moveFromAwaitingMerge = async (
198+
landRequestStatus: LandRequestStatus,
199+
lockId: Date,
200+
dependentsAwaitingMerge: LandRequestStatus[],
201+
) => {
198202
const landRequest = landRequestStatus.request;
199203
const pullRequest = landRequest.pullRequest;
200204
const dependencies = await landRequest.getDependencies();
@@ -213,14 +217,19 @@ export class Runner {
213217

214218
// Try to merge PR
215219
try {
220+
// skip CI if there is a dependent PR that is awaiting merge
221+
const skipCI =
222+
dependentsAwaitingMerge.length > 0 &&
223+
this.config.mergeSettings &&
224+
this.config.mergeSettings.skipBuildOnDependentsAwaitingMerge;
216225
const pullRequestId = landRequest.pullRequestId;
217226
Logger.verbose('Attempting merge pull request', {
218227
namespace: 'lib:runner:moveFromAwaitingMerge',
219228
pullRequestId,
220229
landRequestId: landRequest.id,
221230
lockId,
222231
});
223-
await this.client.mergePullRequest(landRequestStatus);
232+
await this.client.mergePullRequest(landRequestStatus, { skipCI });
224233
Logger.info('Successfully merged PR', {
225234
namespace: 'lib:runner:moveFromAwaitingMerge',
226235
landRequestId: landRequest.id,
@@ -284,7 +293,12 @@ export class Runner {
284293
return landRequest.setStatus('queued', `Queued by ${user.displayName || user.aaid}`);
285294
}
286295
if (landRequestStatus.state === 'awaiting-merge') {
287-
const didChangeState = await this.moveFromAwaitingMerge(landRequestStatus, lockId);
296+
const awaitingMergeQueue = Runner.getDependentsAwaitingMerge(queue, landRequestStatus);
297+
const didChangeState = await this.moveFromAwaitingMerge(
298+
landRequestStatus,
299+
lockId,
300+
awaitingMergeQueue,
301+
);
288302
// if we moved, we need to exit early, otherwise, just keep checking the queue
289303
if (didChangeState) return true;
290304
} else if (landRequestStatus.state === 'queued') {
@@ -729,4 +743,12 @@ export class Runner {
729743
permissionsMessage: this.config.permissionsMessage,
730744
};
731745
};
746+
747+
static getDependentsAwaitingMerge(queue: LandRequestStatus[], currentStatus: LandRequestStatus) {
748+
return queue.filter(
749+
status =>
750+
status.state === 'awaiting-merge' &&
751+
status.request.dependsOn.split(',').includes(String(currentStatus.request.id)),
752+
);
753+
}
732754
}

src/types.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ export type Config = {
9494
sequelize?: any;
9595
eventListeners?: EventListener[];
9696
easterEgg?: any;
97+
mergeSettings?: MergeSettings;
98+
};
99+
100+
export type MergeSettings = {
101+
/** Skip the destination branch build when there are successful dependent requests awaiting merge.
102+
* This prevents multiple branch builds triggering multiple merges happen in quick succession.
103+
* Achieved by adding [skip ci] to the merge commit message
104+
*/
105+
skipBuildOnDependentsAwaitingMerge?: boolean;
106+
// waitForBuild?: { // TBD };
97107
};
98108

99109
export type RunnerState = {
@@ -106,3 +116,7 @@ export type RunnerState = {
106116
bitbucketBaseUrl: string;
107117
permissionsMessage: string;
108118
};
119+
120+
export type MergeOptions = {
121+
skipCI?: boolean;
122+
};

tests/unit/BitbucketAPI.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,17 @@ describe('mergePullRequest', () => {
8686
expect(loggerErrorSpy).toHaveBeenCalledTimes(1);
8787
expect(mockedDelay).toHaveBeenCalledTimes(2);
8888
});
89+
90+
test('Skip-ci merge', async () => {
91+
mockedAxios.post.mockResolvedValue({ status: 200 });
92+
await bitbucketAPI.mergePullRequest(landRequestStatus as any, { skipCI: true });
93+
expect(loggerInfoSpy).toHaveBeenCalledWith(
94+
'Attempting to merge pull request',
95+
expect.objectContaining({
96+
postRequest: expect.objectContaining({
97+
message: expect.stringContaining('[skip ci]'),
98+
}),
99+
}),
100+
);
101+
});
89102
});

0 commit comments

Comments
 (0)