honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(dev-infra): verify breaking changes are properly labeled before merging (#41546) 

During merging with `ng-dev pr merge` tooling will ensure that pull requests are
properly labeled for breaking changes.  Pull requests with commits noting breaking
changes must also be labeled as such, additionally pull requests with breaking
change labels must contain commits noting breaking changes.

Fixes #38776

PR Close #41546
This commit is contained in:
Joey Perrott 2021-04-06 08:57:32 -07:00 committed by Zach Arend
parent 4810f5c819
commit 4a69a7b952
4 changed files with 81 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
dev-infra/ng-dev.js
View File

@ -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

2
dev-infra/pr/merge/config.ts
View File

@ -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.

12
dev-infra/pr/merge/failures.ts
View File

@ -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);
}
}

41
dev-infra/pr/merge/pull-request.ts
View File

@ -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();
}
}