Skip to content

Commit 6f0f0d3

Browse files
devversionAndrewKushnir
authored andcommitted
feat(dev-infra): support user-failures when computing branches for target label (angular#38223)
The merge tool provides a way for configurations to determine the branches for a label lazily. This is supported because it allows labels to respect the currently selected base branch through the Github UI. e.g. if `target: label` is applied on a PR and the PR is based on the patch branch, then the change could only go into the selected target branch, while if it would be based on `master`, the change would be cherry-picked to `master` too. This allows for convenient back-porting of changes if they did not apply cleanly to the primary development branch (`master`). We want to expand this function so that it is possible to report failures if an invalid target label is appplied (e.g. `target: major` not allowed in some situations), or if the Github base branch is not valid for the given target label (e.g. if `target: lts` is used, but it's not based on a LTS branch). PR Close angular#38223
1 parent 576e329 commit 6f0f0d3

File tree

3 files changed

+45
-6
lines changed

3 files changed

+45
-6
lines changed

dev-infra/pr/merge/config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ export interface TargetLabel {
3131
* List of branches a pull request with this target label should be merged into.
3232
* Can also be wrapped in a function that accepts the target branch specified in the
3333
* Github Web UI. This is useful for supporting labels like `target: development-branch`.
34+
*
35+
* @throws {InvalidTargetLabelError} Invalid label has been applied to pull request.
36+
* @throws {InvalidTargetBranchError} Invalid Github target branch has been selected.
3437
*/
3538
branches: TargetLabelBranchResult|((githubTargetBranch: string) => TargetLabelBranchResult);
3639
}

dev-infra/pr/merge/pull-request.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {GitClient} from '../../utils/git';
1212

1313
import {PullRequestFailure} from './failures';
1414
import {matchesPattern} from './string-pattern';
15-
import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest} from './target-label';
15+
import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest, InvalidTargetBranchError, InvalidTargetLabelError} from './target-label';
1616
import {PullRequestMergeTask} from './task';
1717

1818
/** Interface that describes a pull request. */
@@ -83,6 +83,20 @@ export async function loadAndValidatePullRequest(
8383
labels.some(name => matchesPattern(name, config.commitMessageFixupLabel));
8484
const hasCaretakerNote = !!config.caretakerNoteLabel &&
8585
labels.some(name => matchesPattern(name, config.caretakerNoteLabel!));
86+
let targetBranches: string[];
87+
88+
// If branches are determined for a given target label, capture errors that are
89+
// thrown as part of branch computation. This is expected because a merge configuration
90+
// can lazily compute branches for a target label and throw. e.g. if an invalid target
91+
// label is applied, we want to exit the script gracefully with an error message.
92+
try {
93+
targetBranches = await getBranchesFromTargetLabel(targetLabel, githubTargetBranch);
94+
} catch (error) {
95+
if (error instanceof InvalidTargetBranchError || error instanceof InvalidTargetLabelError) {
96+
return new PullRequestFailure(error.failureMessage);
97+
}
98+
throw error;
99+
}
86100

87101
return {
88102
url: prData.html_url,
@@ -92,8 +106,8 @@ export async function loadAndValidatePullRequest(
92106
githubTargetBranch,
93107
needsCommitMessageFixup,
94108
hasCaretakerNote,
109+
targetBranches,
95110
title: prData.title,
96-
targetBranches: getBranchesFromTargetLabel(targetLabel, githubTargetBranch),
97111
commitCount: prData.commits,
98112
};
99113
}

dev-infra/pr/merge/target-label.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@
99
import {MergeConfig, TargetLabel} from './config';
1010
import {matchesPattern} from './string-pattern';
1111

12+
/**
13+
* Unique error that can be thrown in the merge configuration if an
14+
* invalid branch is targeted.
15+
*/
16+
export class InvalidTargetBranchError {
17+
constructor(public failureMessage: string) {}
18+
}
19+
20+
/**
21+
* Unique error that can be thrown in the merge configuration if an
22+
* invalid label has been applied to a pull request.
23+
*/
24+
export class InvalidTargetLabelError {
25+
constructor(public failureMessage: string) {}
26+
}
27+
1228
/** Gets the target label from the specified pull request labels. */
1329
export function getTargetLabelFromPullRequest(config: MergeConfig, labels: string[]): TargetLabel|
1430
null {
@@ -21,8 +37,14 @@ export function getTargetLabelFromPullRequest(config: MergeConfig, labels: strin
2137
return null;
2238
}
2339

24-
/** Gets the branches from the specified target label. */
25-
export function getBranchesFromTargetLabel(
26-
label: TargetLabel, githubTargetBranch: string): string[] {
27-
return typeof label.branches === 'function' ? label.branches(githubTargetBranch) : label.branches;
40+
/**
41+
* Gets the branches from the specified target label.
42+
*
43+
* @throws {InvalidTargetLabelError} Invalid label has been applied to pull request.
44+
* @throws {InvalidTargetBranchError} Invalid Github target branch has been selected.
45+
*/
46+
export async function getBranchesFromTargetLabel(
47+
label: TargetLabel, githubTargetBranch: string): Promise<string[]> {
48+
return typeof label.branches === 'function' ? await label.branches(githubTargetBranch) :
49+
await label.branches;
2850
}

0 commit comments

Comments
 (0)