From 7dba0711c2428c559ab532d8a9567dcd695375d3 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Tue, 6 Apr 2021 07:49:49 -0700 Subject: [PATCH] feat(dev-infra): prevent merges for PRs with invalid breaking changes or commit types (#41459) Check commits in pull requests to ensure the pr can be merged into the target branch. Confirms that prs targeting minor do not contain breaking changes, and prs targeting patch or lts do not contain breaking changes or `feat` commits. PR Close #41459 --- dev-infra/ng-dev.js | 50 ++++++++++++++++++++++++++++++ dev-infra/pr/merge/failures.ts | 15 +++++++++ dev-infra/pr/merge/pull-request.ts | 43 ++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index b25d509224..69807d1308 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -3353,6 +3353,17 @@ var PullRequestFailure = /** @class */ (function () { "your auth token has write access."; } return new this(message); }; + PullRequestFailure.hasBreakingChanges = function (label) { + var message = "Cannot merge into branch for \"" + label.pattern + "\" as the pull request has " + + "breaking changes. Breaking changes can only be merged with the \"target: major\" label."; + return new this(message); + }; + PullRequestFailure.hasFeatureCommits = function (label) { + var message = "Cannot merge into branch for \"" + label.pattern + "\" as the pull request has " + + 'commits with the "feat" type. New features can only be merged with the "target: minor" or ' + + '"target: major" label.'; + return new this(message); + }; return PullRequestFailure; }()); @@ -3412,6 +3423,12 @@ function loadAndValidatePullRequest(_a, prNumber, ignoreNonFatalFailures) { } throw error; } + try { + assertCorrectTargetForChanges(prData.commits.nodes.map(function (n) { return n.commit.message; }), targetLabel); + } + catch (error) { + return [2 /*return*/, error]; + } state = prData.commits.nodes.slice(-1)[0].commit.status.state; if (state === 'FAILURE' && !ignoreNonFatalFailures) { return [2 /*return*/, PullRequestFailure.failingCiJobs()]; @@ -3503,6 +3520,39 @@ function fetchPullRequestFromGithub(git, prNumber) { function isPullRequest(v) { return v.targetBranches !== undefined; } +/** + * Assert the commits provided are allowed to merge to the provided target label, throwing a + * PullRequestFailure otherwise. + */ +function assertCorrectTargetForChanges(rawCommits, label) { + /** List of ParsedCommits for all of the commits in the pull request. */ + var commits = rawCommits.map(parseCommitMessage); + switch (label.pattern) { + case 'target: major': + break; + case 'target: minor': + // Check if any commits in the PR contains a breaking change. + if (commits.some(function (commit) { return commit.breakingChanges.length !== 0; })) { + throw PullRequestFailure.hasBreakingChanges(label); + } + break; + case 'target: patch': + case 'target: lts': + // Check if any commits in the PR contains a breaking change. + if (commits.some(function (commit) { return commit.breakingChanges.length !== 0; })) { + throw PullRequestFailure.hasBreakingChanges(label); + } + // Check if any commits in the PR contains a commit type of "feat". + if (commits.some(function (commit) { return commit.type === 'feat'; })) { + throw PullRequestFailure.hasFeatureCommits(label); + } + break; + default: + warn(red('WARNING: Unable to confirm all commits in the PR are eligible to be merged')); + warn(red("into the target branch: " + label.pattern)); + break; + } +} /** * @license diff --git a/dev-infra/pr/merge/failures.ts b/dev-infra/pr/merge/failures.ts index 043fcaf126..94b6f378b0 100644 --- a/dev-infra/pr/merge/failures.ts +++ b/dev-infra/pr/merge/failures.ts @@ -6,6 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ +import {TargetLabel} from './config'; + /** * Class that can be used to describe pull request failures. A failure * is described through a human-readable message and a flag indicating @@ -72,4 +74,17 @@ export class PullRequestFailure { `your auth token has write access.`) { return new this(message); } + + static hasBreakingChanges(label: TargetLabel) { + const message = `Cannot merge into branch for "${label.pattern}" as the pull request has ` + + `breaking changes. Breaking changes can only be merged with the "target: major" label.`; + return new this(message); + } + + static hasFeatureCommits(label: TargetLabel) { + const message = `Cannot merge into branch for "${label.pattern}" as the pull request has ` + + 'commits with the "feat" type. New features can only be merged with the "target: minor" ' + + 'or "target: major" label.'; + return new this(message); + } } diff --git a/dev-infra/pr/merge/pull-request.ts b/dev-infra/pr/merge/pull-request.ts index b1f7ef877a..e3ff09fc63 100644 --- a/dev-infra/pr/merge/pull-request.ts +++ b/dev-infra/pr/merge/pull-request.ts @@ -7,6 +7,8 @@ */ import {params, types as graphQLTypes} from 'typed-graphqlify'; +import {parseCommitMessage} from '../../commit-message/parse'; +import {red, warn} from '../../utils/console'; import {GitClient} from '../../utils/git/index'; import {getPr} from '../../utils/github'; @@ -73,6 +75,11 @@ export async function loadAndValidatePullRequest( throw error; } + try { + assertCorrectTargetForChanges(prData.commits.nodes.map(n => n.commit.message), targetLabel); + } catch (error) { + return error; + } const state = prData.commits.nodes.slice(-1)[0].commit.status.state; if (state === 'FAILURE' && !ignoreNonFatalFailures) { @@ -126,7 +133,7 @@ const PR_SCHEMA = { nodes: [{ commit: { status: { - state: graphQLTypes.oneOf(['FAILURE' as const, 'PENDING' as const, 'SUCCESS' as const]), + state: graphQLTypes.oneOf(['FAILURE', 'PENDING', 'SUCCESS'] as const), }, message: graphQLTypes.string, }, @@ -162,3 +169,37 @@ async function fetchPullRequestFromGithub( export function isPullRequest(v: PullRequestFailure|PullRequest): v is PullRequest { return (v as PullRequest).targetBranches !== undefined; } + +/** + * Assert the commits provided are allowed to merge to the provided target label, throwing a + * PullRequestFailure otherwise. + */ +function assertCorrectTargetForChanges(rawCommits: string[], label: TargetLabel) { + /** List of ParsedCommits for all of the commits in the pull request. */ + const commits = rawCommits.map(parseCommitMessage); + switch (label.pattern) { + case 'target: major': + break; + case 'target: minor': + // Check if any commits in the PR contains a breaking change. + if (commits.some(commit => commit.breakingChanges.length !== 0)) { + throw PullRequestFailure.hasBreakingChanges(label); + } + break; + case 'target: patch': + case 'target: lts': + // Check if any commits in the PR contains a breaking change. + if (commits.some(commit => commit.breakingChanges.length !== 0)) { + throw PullRequestFailure.hasBreakingChanges(label); + } + // Check if any commits in the PR contains a commit type of "feat". + if (commits.some(commit => commit.type === 'feat')) { + throw PullRequestFailure.hasFeatureCommits(label); + } + break; + default: + warn(red('WARNING: Unable to confirm all commits in the PR are eligible to be merged')); + warn(red(`into the target branch: ${label.pattern}`)); + break; + } +}