From cc3a89b08a087b8e064f1b5c67b63afd2db65d68 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Fri, 24 Sep 2021 14:35:57 -0700 Subject: [PATCH] refactor(ng-dev/pr): move merge ready assertion to common valiations Move the merge ready label check validation to the common validations. --- ng-dev/pr/common/validation/validations.ts | 17 +++++++++++++++++ ng-dev/pr/merge/pull-request.ts | 8 +++----- ng-dev/pr/merge/strategies/api-merge.ts | 2 +- ng-dev/pr/merge/string-pattern.ts | 12 ------------ 4 files changed, 21 insertions(+), 18 deletions(-) delete mode 100644 ng-dev/pr/merge/string-pattern.ts diff --git a/ng-dev/pr/common/validation/validations.ts b/ng-dev/pr/common/validation/validations.ts index 0e8f982e9..bfe280712 100644 --- a/ng-dev/pr/common/validation/validations.ts +++ b/ng-dev/pr/common/validation/validations.ts @@ -122,3 +122,20 @@ export function assertSignedCla(pullRequest: PullRequestFromGithub) { throw PullRequestFailure.claUnsigned(); } + +/** + * Assert the pull request has been marked ready for merge by the author. + * @throws {PullRequestFailure} if the pull request is missing the merge ready label. + */ +export function assertMergeReady(pullRequest: PullRequestFromGithub, config: PullRequestConfig) { + if (pullRequest.labels.nodes.some(({name}) => matchesPattern(name, config.mergeReadyLabel))) { + return true; + } + throw PullRequestFailure.notMergeReady(); +} + +// TODO: Remove need to export this pattern matching utility. +/** Checks whether the specified value matches the given pattern. */ +export function matchesPattern(value: string, pattern: RegExp | string): boolean { + return typeof pattern === 'string' ? value === pattern : pattern.test(value); +} diff --git a/ng-dev/pr/merge/pull-request.ts b/ng-dev/pr/merge/pull-request.ts index decd602ae..938e0991d 100644 --- a/ng-dev/pr/merge/pull-request.ts +++ b/ng-dev/pr/merge/pull-request.ts @@ -8,11 +8,12 @@ import {parseCommitMessage} from '../../commit-message/parse'; import {PullRequestFailure} from '../common/validation/failures'; -import {matchesPattern} from './string-pattern'; import {PullRequestMergeTask} from './task'; import {getTargetBranchesForPullRequest} from '../common/targeting/target-label'; import { assertCorrectBreakingChangeLabeling, + assertMergeReady, + matchesPattern, assertPendingState, assertSignedCla, } from '../common/validation/validations'; @@ -63,10 +64,6 @@ export async function loadAndValidatePullRequest( const labels = prData.labels.nodes.map((l) => l.name); - if (!labels.some((name) => matchesPattern(name, config.pullRequest.mergeReadyLabel))) { - return PullRequestFailure.notMergeReady(); - } - /** List of parsed commits for all of the commits in the pull request. */ const commitsInPr = prData.commits.nodes.map((n) => parseCommitMessage(n.commit.message)); const githubTargetBranch = prData.baseRefName; @@ -80,6 +77,7 @@ export async function loadAndValidatePullRequest( ); try { + assertMergeReady(prData, config.pullRequest); assertSignedCla(prData); assertPendingState(prData); assertCorrectBreakingChangeLabeling(commitsInPr, labels); diff --git a/ng-dev/pr/merge/strategies/api-merge.ts b/ng-dev/pr/merge/strategies/api-merge.ts index 450631a44..667ccac15 100644 --- a/ng-dev/pr/merge/strategies/api-merge.ts +++ b/ng-dev/pr/merge/strategies/api-merge.ts @@ -14,10 +14,10 @@ import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-clien import {GithubApiMergeMethod, GithubApiMergeStrategyConfig} from '../../config'; import {PullRequestFailure} from '../../common/validation/failures'; import {PullRequest} from '../pull-request'; -import {matchesPattern} from '../string-pattern'; import {MergeStrategy, TEMP_PR_HEAD_BRANCH} from './strategy'; import {GithubApiRequestError} from '../../../utils/git/github'; +import {matchesPattern} from '../../common/validation/validations'; /** Type describing the parameters for the Octokit `merge` API endpoint. */ type OctokitMergeParams = RestEndpointMethodTypes['pulls']['merge']['parameters']; diff --git a/ng-dev/pr/merge/string-pattern.ts b/ng-dev/pr/merge/string-pattern.ts deleted file mode 100644 index 925c9e592..000000000 --- a/ng-dev/pr/merge/string-pattern.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * 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 - */ - -/** Checks whether the specified value matches the given pattern. */ -export function matchesPattern(value: string, pattern: RegExp | string): boolean { - return typeof pattern === 'string' ? value === pattern : pattern.test(value); -}