Skip to content

Commit 95499f4

Browse files
committed
feat(ng-dev): add conditional autosquash merge strategy
Introduces a new conditional autosquash merge strategy. This strategy uses the autosquash merge strategy if the pull request contains fixup or squash commits, and the GitHub API merge strategy otherwise. This allows pull requests that do not need to be squashed to be closed as "merged" on GitHub, rather than "closed".
1 parent c584c35 commit 95499f4

File tree

6 files changed

+119
-23
lines changed

6 files changed

+119
-23
lines changed

.ng-dev/pull-request.mts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import {PullRequestConfig} from '../ng-dev/pr/config/index.js';
22

33
/** Configuration for interacting with pull requests in the repo. */
44
export const pullRequest: PullRequestConfig = {
5-
githubApiMerge: false,
5+
githubApiMerge: {
6+
default: 'rebase',
7+
labels: [{pattern: 'merge: squash commits', method: 'squash'}],
8+
},
9+
conditionalAutosquashMerge: true,
610
requiredStatuses: [{name: 'test', type: 'check'}],
711

812
// Disable target labeling in the dev-infra repo as we don't have

ng-dev/pr/config/index.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export interface GithubApiMergeStrategyConfig {
2323
}
2424

2525
/** Configuration for the merge script. */
26-
export interface PullRequestConfig {
26+
export type PullRequestConfig = {
2727
/**
2828
* Configuration for the upstream remote. All of these options are optional as
2929
* defaults are provided by the common dev-infra github configuration.
@@ -63,7 +63,25 @@ export interface PullRequestConfig {
6363
* follow the canonical branching/versioning.
6464
*/
6565
__noTargetLabeling?: boolean;
66-
}
66+
} & {
67+
/**
68+
* Whether pull requests should be merged using a conditional autosquash strategy.
69+
* If a pull request contains fixup or squash commits, the autosquash strategy
70+
* will be used. Otherwise, the Github API merge strategy will be used.
71+
*/
72+
conditionalAutosquashMerge?: boolean;
73+
74+
/**
75+
* The configuration for merging pull requests using the Github API.
76+
*
77+
* This strategy is used as a fallback for the `conditionalAutosquashMerge` strategy,
78+
* when a pull request does not contain any fixup or squash commits.
79+
*
80+
* This can be enabled if projects want to have their pull requests show up as
81+
* `Merged` in the Github UI.
82+
*/
83+
githubApiMerge: GithubApiMergeStrategyConfig;
84+
};
6785

6886
/** Loads and validates the merge configuration. */
6987
export function assertValidPullRequestConfig<T extends NgDevConfig>(
@@ -76,10 +94,18 @@ export function assertValidPullRequestConfig<T extends NgDevConfig>(
7694
);
7795
}
7896

79-
if (config.pullRequest.githubApiMerge === undefined) {
97+
const {conditionalAutosquashMerge, githubApiMerge} = config.pullRequest;
98+
if (githubApiMerge === undefined) {
8099
errors.push('No explicit choice of merge strategy. Please set `githubApiMerge`.');
81100
}
82101

102+
if (conditionalAutosquashMerge && !githubApiMerge) {
103+
errors.push(
104+
'`conditionalAutosquashMerge` requires a GitHub API merge strategy to inspect commit history. ' +
105+
'Please configure `githubApiMerge` or disable `conditionalAutosquashMerge`.',
106+
);
107+
}
108+
83109
if (errors.length) {
84110
throw new ConfigValidationError('Invalid `pullRequest` configuration', errors);
85111
}

ng-dev/pr/merge/merge-tool.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import {loadAndValidatePullRequest, PullRequest} from './pull-request.js';
2020
import {GithubApiMergeStrategy} from './strategies/api-merge.js';
2121
import {AutosquashMergeStrategy} from './strategies/autosquash-merge.js';
22+
import {ConditionalAutosquashMergeStrategy} from './strategies/conditional-autosquash-merge.js';
2223
import {GithubConfig, NgDevConfig} from '../../utils/config.js';
2324
import {assertValidReleaseConfig} from '../../release/config/index.js';
2425
import {
@@ -139,9 +140,20 @@ export class MergeTool {
139140
throw new UserAbortedMergeToolError();
140141
}
141142

142-
const strategy = this.config.pullRequest.githubApiMerge
143-
? new GithubApiMergeStrategy(this.git, this.config.pullRequest.githubApiMerge)
144-
: new AutosquashMergeStrategy(this.git);
143+
let strategy:
144+
| AutosquashMergeStrategy
145+
| ConditionalAutosquashMergeStrategy
146+
| GithubApiMergeStrategy;
147+
148+
const {conditionalAutosquashMerge, githubApiMerge} = this.config.pullRequest;
149+
150+
if (conditionalAutosquashMerge) {
151+
strategy = new ConditionalAutosquashMergeStrategy(this.git, githubApiMerge);
152+
} else if (githubApiMerge) {
153+
strategy = new GithubApiMergeStrategy(this.git, githubApiMerge);
154+
} else {
155+
strategy = new AutosquashMergeStrategy(this.git);
156+
}
145157

146158
// Branch or revision that is currently checked out so that we can switch back to
147159
// it once the pull request has been merged.

ng-dev/pr/merge/strategies/api-merge.ts

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import {RestEndpointMethodTypes} from '@octokit/plugin-rest-endpoint-methods';
1010

11-
import {parseCommitMessage} from '../../../commit-message/parse.js';
1211
import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
1312
import {GithubApiMergeMethod, GithubApiMergeStrategyConfig} from '../../config/index.js';
1413
import {PullRequest} from '../pull-request.js';
@@ -52,7 +51,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
5251
*/
5352
override async merge(pullRequest: PullRequest): Promise<void> {
5453
const {githubTargetBranch, prNumber, needsCommitMessageFixup, targetBranches} = pullRequest;
55-
const method = this._getMergeActionFromPullRequest(pullRequest);
54+
const method = this.getMergeActionFromPullRequest(pullRequest);
5655
const cherryPickTargetBranches = targetBranches.filter((b) => b !== githubTargetBranch);
5756

5857
const mergeOptions: OctokitMergeParams = {
@@ -195,10 +194,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
195194
* behavior here so that we have a default commit message that can be fixed up.
196195
*/
197196
private async _getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise<string> {
198-
const commits = (await this._getPullRequestCommitMessages(pullRequest)).map((message) => ({
199-
message,
200-
parsed: parseCommitMessage(message),
201-
}));
197+
const commits = await this.getPullRequestCommits(pullRequest);
202198
const messageBase = `${pullRequest.title}${COMMIT_HEADER_SEPARATOR}`;
203199
if (commits.length <= 1) {
204200
return `${messageBase}${commits[0].parsed.body}`;
@@ -207,17 +203,8 @@ export class GithubApiMergeStrategy extends MergeStrategy {
207203
return `${messageBase}${joinedMessages}`;
208204
}
209205

210-
/** Gets all commit messages of commits in the pull request. */
211-
private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
212-
const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
213-
...this.git.remoteParams,
214-
pull_number: prNumber,
215-
});
216-
return allCommits.map(({commit}) => commit.message);
217-
}
218-
219206
/** Determines the merge action from the given pull request. */
220-
private _getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
207+
getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
221208
if (this._config.labels) {
222209
const matchingLabel = this._config.labels.find(({pattern}) => labels.includes(pattern));
223210
if (matchingLabel !== undefined) {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
10+
import {GithubApiMergeStrategyConfig} from '../../config/index.js';
11+
import {PullRequest} from '../pull-request.js';
12+
13+
import {AutosquashMergeStrategy} from './autosquash-merge.js';
14+
import {GithubApiMergeStrategy} from './api-merge.js';
15+
import {MergeStrategy} from './strategy.js';
16+
17+
/**
18+
* Merge strategy that conditionally uses autosquash or the Github API merge.
19+
* If a pull request contains fixup or squash commits, the autosquash strategy
20+
* will be used. Otherwise, the Github API merge strategy will be used.
21+
*/
22+
export class ConditionalAutosquashMergeStrategy extends MergeStrategy {
23+
private readonly githubApiMergeStrategy: GithubApiMergeStrategy;
24+
25+
constructor(
26+
git: AuthenticatedGitClient,
27+
private config: GithubApiMergeStrategyConfig,
28+
) {
29+
super(git);
30+
this.githubApiMergeStrategy = new GithubApiMergeStrategy(this.git, this.config);
31+
}
32+
33+
override async merge(pullRequest: PullRequest): Promise<void> {
34+
const mergeAction = this.githubApiMergeStrategy.getMergeActionFromPullRequest(pullRequest);
35+
36+
// Squash and Merge will create a single commit message and thus we can use the API to merge.
37+
return mergeAction === 'rebase' && (await this.hasFixupOrSquashCommits(pullRequest))
38+
? new AutosquashMergeStrategy(this.git).merge(pullRequest)
39+
: this.githubApiMergeStrategy.merge(pullRequest);
40+
}
41+
42+
/** Checks whether the pull request contains fixup or squash commits. */
43+
private async hasFixupOrSquashCommits(pullRequest: PullRequest): Promise<boolean> {
44+
const commits = await this.getPullRequestCommits(pullRequest);
45+
46+
return commits.some(({parsed: {isFixup, isSquash}}) => isFixup || isSquash);
47+
}
48+
}

ng-dev/pr/merge/strategies/strategy.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {Commit, parseCommitMessage} from '../../../commit-message/parse.js';
910
import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
1011
import {
1112
FatalMergeToolError,
@@ -200,4 +201,22 @@ export abstract class MergeStrategy {
200201
throw new MergeConflictsFatalError(failedBranches);
201202
}
202203
}
204+
205+
/** Gets all commit messages of commits in the pull request. */
206+
protected async getPullRequestCommits({prNumber}: PullRequest): Promise<
207+
{
208+
message: string;
209+
parsed: Commit;
210+
}[]
211+
> {
212+
const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
213+
...this.git.remoteParams,
214+
pull_number: prNumber,
215+
});
216+
217+
return allCommits.map(({commit: {message}}) => ({
218+
message,
219+
parsed: parseCommitMessage(message),
220+
}));
221+
}
203222
}

0 commit comments

Comments
 (0)