diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index 0ceb6c45bb..5d86f09a9f 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -3365,6 +3365,16 @@ var PullRequestFailure = /** @class */ (function () { 'or "target: major" label.'; return new this(message); }; + PullRequestFailure.missingBreakingChangeLabel = function () { + var message = 'Pull Request has at least one commit containing a breaking change note, but ' + + 'does not have a breaking change label.'; + return new this(message); + }; + PullRequestFailure.missingBreakingChangeCommit = function () { + var message = 'Pull Request has a breaking change label, but does not contain any commits ' + + 'with breaking change notes.'; + return new this(message); + }; return PullRequestFailure; }()); @@ -3391,6 +3401,8 @@ function getTargettedBranchesConfirmationPromptMessage(pullRequest) { * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +/** The default label for labeling pull requests containing a breaking change. */ +var BreakingChangeLabel = 'breaking changes'; /** * Loads and validates the specified pull request against the given configuration. * If the pull requests fails, a pull request failure is returned. @@ -3399,7 +3411,7 @@ function loadAndValidatePullRequest(_a, prNumber, ignoreNonFatalFailures) { var git = _a.git, config = _a.config; if (ignoreNonFatalFailures === void 0) { ignoreNonFatalFailures = false; } return tslib.__awaiter(this, void 0, void 0, function () { - var prData, labels, targetLabel, commitMessages, state, githubTargetBranch, requiredBaseSha, needsCommitMessageFixup, hasCaretakerNote, targetBranches, error_1; + var prData, labels, targetLabel, commitsInPr, state, githubTargetBranch, requiredBaseSha, needsCommitMessageFixup, hasCaretakerNote, targetBranches, error_1; return tslib.__generator(this, function (_b) { switch (_b.label) { case 0: return [4 /*yield*/, fetchPullRequestFromGithub(git, prNumber)]; @@ -3424,9 +3436,10 @@ function loadAndValidatePullRequest(_a, prNumber, ignoreNonFatalFailures) { } throw error; } + commitsInPr = prData.commits.nodes.map(function (n) { return parseCommitMessage(n.commit.message); }); try { - commitMessages = prData.commits.nodes.map(function (n) { return n.commit.message; }); - assertChangesAllowForTargetLabel(commitMessages, targetLabel, config); + assertChangesAllowForTargetLabel(commitsInPr, targetLabel, config); + assertCorrectBreakingChangeLabeling(commitsInPr, labels, config); } catch (error) { return [2 /*return*/, error]; @@ -3531,16 +3544,14 @@ function isPullRequest(v) { * Assert the commits provided are allowed to merge to the provided target label, throwing a * PullRequestFailure otherwise. */ -function assertChangesAllowForTargetLabel(rawCommits, label, config) { +function assertChangesAllowForTargetLabel(commits, label, config) { /** * List of commit scopes which are exempted from target label content requirements. i.e. no `feat` * scopes in patch branches, no breaking changes in minor or patch changes. */ var exemptedScopes = config.targetLabelExemptScopes || []; - /** List of parsed commits which are subject to content requirements for the target label. */ - var commits = rawCommits.map(parseCommitMessage).filter(function (commit) { - return !exemptedScopes.includes(commit.scope); - }); + /** List of commits which are subject to content requirements for the target label. */ + commits = commits.filter(function (commit) { return !exemptedScopes.includes(commit.scope); }); switch (label.pattern) { case 'target: major': break; @@ -3567,6 +3578,22 @@ function assertChangesAllowForTargetLabel(rawCommits, label, config) { break; } } +/** + * Assert the pull request has the proper label for breaking changes if there are breaking change + * commits, and only has the label if there are breaking change commits. + */ +function assertCorrectBreakingChangeLabeling(commits, labels, config) { + /** Whether the PR has a label noting a breaking change. */ + var hasLabel = labels.includes(config.breakingChangeLabel || BreakingChangeLabel); + //** Whether the PR has at least one commit which notes a breaking change. */ + var hasCommit = commits.some(function (commit) { return commit.breakingChanges.length !== 0; }); + if (!hasLabel && hasCommit) { + throw PullRequestFailure.missingBreakingChangeLabel(); + } + if (hasLabel && !hasCommit) { + throw PullRequestFailure.missingBreakingChangeCommit(); + } +} /** * @license diff --git a/dev-infra/pr/merge/config.ts b/dev-infra/pr/merge/config.ts index 03cea76aaa..b34df2ed24 100644 --- a/dev-infra/pr/merge/config.ts +++ b/dev-infra/pr/merge/config.ts @@ -64,6 +64,8 @@ export interface MergeConfig { caretakerNoteLabel?: string|RegExp; /** Label which can be applied to fixup commit messages in the merge script. */ commitMessageFixupLabel: string|RegExp; + /** Label that is applied when a breaking change is made in the pull request. */ + breakingChangeLabel?: string; /** * Whether pull requests should be merged using the Github API. This can be enabled * if projects want to have their pull requests show up as `Merged` in the Github UI. diff --git a/dev-infra/pr/merge/failures.ts b/dev-infra/pr/merge/failures.ts index 01aa6ae2ee..5675eb5ad0 100644 --- a/dev-infra/pr/merge/failures.ts +++ b/dev-infra/pr/merge/failures.ts @@ -87,4 +87,16 @@ export class PullRequestFailure { 'or "target: major" label.'; return new this(message); } + + static missingBreakingChangeLabel() { + const message = 'Pull Request has at least one commit containing a breaking change note, but ' + + 'does not have a breaking change label.'; + return new this(message); + } + + static missingBreakingChangeCommit() { + const message = 'Pull Request has a breaking change label, but does not contain any commits ' + + 'with breaking change notes.'; + return new this(message); + } } diff --git a/dev-infra/pr/merge/pull-request.ts b/dev-infra/pr/merge/pull-request.ts index c79435103d..07988287b9 100644 --- a/dev-infra/pr/merge/pull-request.ts +++ b/dev-infra/pr/merge/pull-request.ts @@ -7,7 +7,7 @@ */ import {params, types as graphQLTypes} from 'typed-graphqlify'; -import {parseCommitMessage} from '../../commit-message/parse'; +import {Commit, parseCommitMessage} from '../../commit-message/parse'; import {red, warn} from '../../utils/console'; import {GitClient} from '../../utils/git/index'; @@ -19,6 +19,9 @@ import {matchesPattern} from './string-pattern'; import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest, InvalidTargetBranchError, InvalidTargetLabelError} from './target-label'; import {PullRequestMergeTask} from './task'; +/** The default label for labeling pull requests containing a breaking change. */ +const BreakingChangeLabel = 'breaking changes'; + /** Interface that describes a pull request. */ export interface PullRequest { /** URL to the pull request. */ @@ -75,10 +78,12 @@ export async function loadAndValidatePullRequest( throw error; } + /** List of parsed commits for all of the commits in the pull request. */ + const commitsInPr = prData.commits.nodes.map(n => parseCommitMessage(n.commit.message)); + try { - /** Commit message strings for all commits in the pull request. */ - const commitMessages = prData.commits.nodes.map(n => n.commit.message); - assertChangesAllowForTargetLabel(commitMessages, targetLabel, config); + assertChangesAllowForTargetLabel(commitsInPr, targetLabel, config); + assertCorrectBreakingChangeLabeling(commitsInPr, labels, config); } catch (error) { return error; } @@ -182,16 +187,14 @@ export function isPullRequest(v: PullRequestFailure|PullRequest): v is PullReque * PullRequestFailure otherwise. */ function assertChangesAllowForTargetLabel( - rawCommits: string[], label: TargetLabel, config: MergeConfig) { + commits: Commit[], label: TargetLabel, config: MergeConfig) { /** * List of commit scopes which are exempted from target label content requirements. i.e. no `feat` * scopes in patch branches, no breaking changes in minor or patch changes. */ const exemptedScopes = config.targetLabelExemptScopes || []; - /** List of parsed commits which are subject to content requirements for the target label. */ - let commits = rawCommits.map(parseCommitMessage).filter(commit => { - return !exemptedScopes.includes(commit.scope); - }); + /** List of commits which are subject to content requirements for the target label. */ + commits = commits.filter(commit => !exemptedScopes.includes(commit.scope)); switch (label.pattern) { case 'target: major': break; @@ -218,3 +221,23 @@ function assertChangesAllowForTargetLabel( break; } } + +/** + * Assert the pull request has the proper label for breaking changes if there are breaking change + * commits, and only has the label if there are breaking change commits. + */ +function assertCorrectBreakingChangeLabeling( + commits: Commit[], labels: string[], config: MergeConfig) { + /** Whether the PR has a label noting a breaking change. */ + const hasLabel = labels.includes(config.breakingChangeLabel || BreakingChangeLabel); + //** Whether the PR has at least one commit which notes a breaking change. */ + const hasCommit = commits.some(commit => commit.breakingChanges.length !== 0); + + if (!hasLabel && hasCommit) { + throw PullRequestFailure.missingBreakingChangeLabel(); + } + + if (hasLabel && !hasCommit) { + throw PullRequestFailure.missingBreakingChangeCommit(); + } +}