From df49fd6ad8df532f753f91783d4fd8095813c360 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Sat, 3 Oct 2020 13:57:36 -0400 Subject: [PATCH] build: remove existing commit message validation The shared dev-infra toolset is now used to perform commit message validation. --- package.json | 4 +- scripts/README.md | 2 +- scripts/hooks/pre-push.ts | 44 -------- scripts/validate-commits.ts | 210 ------------------------------------ scripts/validate.ts | 6 -- 5 files changed, 2 insertions(+), 264 deletions(-) delete mode 100644 scripts/hooks/pre-push.ts delete mode 100644 scripts/validate-commits.ts diff --git a/package.json b/package.json index 998935f7e3..acd2e9c430 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,6 @@ "lint": "tslint --project tsconfig.json", "templates": "node ./bin/devkit-admin templates", "validate": "node ./bin/devkit-admin validate", - "validate-commits": "./bin/devkit-admin validate-commits", "preinstall": "node ./tools/yarn/check-yarn.js", "postinstall": "yarn webdriver-update && yarn ngcc", "//webdriver-update-README": "ChromeDriver version must match Puppeteer Chromium version, see https://github.com/GoogleChrome/puppeteer/releases http://chromedriver.chromium.org/downloads", @@ -240,8 +239,7 @@ "husky": { "hooks": { "commit-msg": "yarn -s ng-dev commit-message pre-commit-validate --file-env-variable HUSKY_GIT_PARAMS", - "prepare-commit-msg": "yarn -s ng-dev commit-message restore-commit-message-draft --file-env-variable HUSKY_GIT_PARAMS", - "pre-push": "node ./bin/devkit-admin hooks/pre-push" + "prepare-commit-msg": "yarn -s ng-dev commit-message restore-commit-message-draft --file-env-variable HUSKY_GIT_PARAMS" } } } diff --git a/scripts/README.md b/scripts/README.md index f970712d4a..8825308239 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -123,7 +123,7 @@ Run integration tests using Bazel. ## validate -Performs BUILD files, commit messages and license validation. +Performs BUILD files and license validation. Flags: diff --git a/scripts/hooks/pre-push.ts b/scripts/hooks/pre-push.ts deleted file mode 100644 index b68b2ec073..0000000000 --- a/scripts/hooks/pre-push.ts +++ /dev/null @@ -1,44 +0,0 @@ -/** - * @license - * Copyright Google Inc. 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 - */ -// tslint:disable:no-any -// tslint:disable:no-implicit-dependencies -import { logging } from '@angular-devkit/core'; -import * as readline from 'readline'; -import validateCommits from '../validate-commits'; - - -const emptySha = '0'.repeat(40); - - -export default function (_: {}, logger: logging.Logger) { - let validateCommitResult = 0; - - // Work on POSIX and Windows - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - terminal: false, - }); - - rl.on('line', line => { - const [, localSha, , remoteSha] = line.split(/\s+/); - - if (localSha == emptySha) { - // Deleted branch. - return; - } - - if (remoteSha == emptySha) { - // New branch. - validateCommitResult = validateCommits({ base: localSha }, logger); - } else { - validateCommitResult = validateCommits({ base: remoteSha, head: localSha }, logger); - } - }); - rl.on('end', () => process.exit(validateCommitResult)); -} diff --git a/scripts/validate-commits.ts b/scripts/validate-commits.ts deleted file mode 100644 index b3ea500fde..0000000000 --- a/scripts/validate-commits.ts +++ /dev/null @@ -1,210 +0,0 @@ -/** - * @license - * Copyright Google Inc. 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 - */ -// tslint:disable:no-implicit-dependencies -import { logging } from '@angular-devkit/core'; -import { execSync } from 'child_process'; -import { packages } from '../lib/packages'; - - -const ignoreCommitShaList = [ - '9ce1aed331ad0742463b587f1f5555486ccc202f', - 'de7a44f23514594274394322adaf40ac87c38d8b', - '21d87e93955b12d137417a2bb0536d7faef3ad07', - '76a3ec3fea0482e15851dda1c61bb90b8c643bb7', - '09d019e73308f6914a94f99284f7d54063da420e', - '012672161087a05ae5ecffbed5d1ee307ce1e0ad', - 'dd39597b6a061100cce16494613332368b0f2553', - 'a11dddf454590c8df7e8fbc500e589eafbcb48fe', - '9a9bc00a453988469b12a434372021a812e11223', - '167f6fb95843e80a650ff948361c8c24bfc302d8', - 'fb1c2af810d069229760800c2f539e8a764fe1eb', - '7ba94c8084eb65407abd5531dcbb7275401969c9', - '8475b3dadba3d50a96a2f876188bbf78d55039aa', - 'bf566c710181b0e23af0070540294de99f259fe9', - 'fa6795a8471d1c9ae4d583f8e698e49f74a137e6', -]; - - -export enum Scope { - MustHave = 0, - MustNotHave = 1, - Either = 2, -} - -export const types: { [t: string]: { description: string, scope: Scope } } = { - // Types that can contain both a scope or no scope. - 'docs': { - description: 'Documentation only changes.', - scope: Scope.Either, - }, - 'refactor': { - description: 'A code change that neither fixes a bug nor adds a feature', - scope: Scope.Either, - }, - 'style': { - description: 'Changes that do not affect the meaning of the code (white-space, formatting, ' - + 'missing semi-colons, etc).', - scope: Scope.Either, - }, - 'test': { - description: 'Adding missing tests or correcting existing tests.', - scope: Scope.Either, - }, - - // Types that MUST contain a scope. - 'feat': { - description: 'A new feature.', - scope: Scope.MustHave, - }, - 'fix': { - description: 'A bug fix.', - scope: Scope.MustHave, - }, - - // Types that MUST NOT contain a scope. - 'build': { - description: 'Changes that affect the build system or external dependencies.', - scope: Scope.MustNotHave, - }, - 'revert': { - description: 'A git commit revert. The description must include the original commit message.', - scope: Scope.MustNotHave, - }, - 'ci': { - description: 'Changes to our CI configuration files and scripts.', - scope: Scope.MustNotHave, - }, - 'release': { - description: 'A release commit. Must only include version changes.', - scope: Scope.MustNotHave, - }, -}; - - -export interface ValidateCommitsOptions { - ci?: boolean; - base?: string; - head?: string; -} - - -export default function (argv: ValidateCommitsOptions, logger: logging.Logger) { - logger.info('Getting merge base...'); - - const prNumber = process.env['CIRCLE_PR_NUMBER'] || ''; - let baseSha = ''; - let sha = ''; - - if (prNumber) { - const url = `https://api.github.com/repos/angular/angular-cli/pulls/${prNumber}`; - const prJson = JSON.parse(execSync(`curl "${url}"`, { - stdio: ['ignore', 'pipe', 'ignore'], - encoding: 'utf8', - }).toString()); - baseSha = prJson['base']['sha']; - sha = prJson['head']['sha']; - } else if (argv.base) { - baseSha = argv.base; - sha = argv.head || 'HEAD'; - } else { - const parentRemote = process.env['GIT_REMOTE'] ? process.env['GIT_REMOTE'] + '/' : ''; - const parentBranch = process.env['GIT_BRANCH'] || 'master'; - baseSha = execSync(`git merge-base --fork-point "${parentRemote}${parentBranch}"`) - .toString().trim(); - sha = 'HEAD'; - } - - logger.createChild('sha').info(`Base: ${baseSha}\nHEAD: ${sha}`); - - const log = execSync(`git log --oneline "${baseSha}..${sha}"`).toString().trim(); - logger.debug('Commits:'); - logger.createChild('commits').debug(log); - logger.debug(''); - - const commits = log.split(/\n/) - .map(i => i.match(/(^[0-9a-f]+) (.+)$/)) - .map(x => x ? Array.from(x).slice(1) : null) - .filter(x => !!x) as [string, string][]; - logger.info(`Found ${commits.length} commits...`); - - const output = logger.createChild('check'); - let invalidCount = 0; - - function _invalid(sha: string, message: string, error: string) { - invalidCount++; - output.error(`The following commit ${error}:`); - output.error(` ${sha} ${message}`); - } - - for (const [sha, message] of commits) { - if (ignoreCommitShaList.find(i => i.startsWith(sha))) { - // Some commits are better ignored. - continue; - } - - const subject = message.match(/^([^:(]+)(?:\((.*?)\))?:/); - if (!subject) { - _invalid(sha, message, 'does not have a subject'); - continue; - } - - const [type, scope] = subject.slice(1); - const validTypes = Object.keys(types).join(', '); - const validScopes = Object.keys(packages).join(', '); - if (!(type in types)) { - _invalid(sha, message, `has an unknown type. Valid types are [${validTypes}]. You can use wip: to avoid this.`); - continue; - } - switch (types[type].scope) { - case Scope.Either: - if (scope && !packages[scope]) { - _invalid(sha, message, `has a scope that does not exist. Valid scopes are [${validScopes}].`); - continue; - } - break; - - case Scope.MustHave: - if (!scope) { - _invalid(sha, message, `should always have a scope. Valid scopes are [${validScopes}].`); - continue; - } - if (!packages[scope]) { - _invalid(sha, message, `has a scope that does not exist. Valid scopes are [${validScopes}].`); - continue; - } - break; - - case Scope.MustNotHave: - if (scope) { - _invalid(sha, message, 'should not have a scope'); - continue; - } - break; - } - - // Custom validation. - if (type == 'release') { - if (argv.ci && commits.length > 1) { - _invalid(sha, message, 'release should always be alone in a PR'); - continue; - } - } else if (type == 'wip') { - if (argv.ci) { - _invalid(sha, message, 'wip are not allowed in a PR'); - } - } - } - - if (invalidCount > 0) { - logger.fatal(`${invalidCount} commits were found invalid...`); - } else { - logger.info('All green. Thank you, come again.'); - } - - return invalidCount; -} diff --git a/scripts/validate.ts b/scripts/validate.ts index 9167a5c08b..9cb76a1ef2 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -10,7 +10,6 @@ import { logging, tags } from '@angular-devkit/core'; import { execSync } from 'child_process'; import templates from './templates'; import validateBuildFiles from './validate-build-files'; -import validateCommits from './validate-commits'; import validateDoNotSubmit from './validate-do-not-submit'; import validateLicenses from './validate-licenses'; import validateUserAnalytics from './validate-user-analytics'; @@ -41,11 +40,6 @@ export default async function (options: { verbose: boolean; ci: boolean }, logge } if (!options.ci) { - logger.info(''); - logger.info('Running commit validation...'); - error = validateCommits({}, logger.createChild('validate-commits')) != 0 - || error; - logger.info(''); logger.info(`Running DO_NOT${''}_SUBMIT validation...`); error = await validateDoNotSubmit({}, logger.createChild('validate-do-not-submit')) != 0