fix(dev-infra): use commit message validation from @angular/dev-infra-private (#36172)
Prior to this change we manage a local version of commit message validation in addition to the commit message validation tool contained in the ng-dev tooling. By adding the ability to validate a range of commit messages together, the remaining piece of commit message validation that is in the local version is replicated. We use both commands provided by the `ng-dev commit-message` tooling: - pre-commit-validate: Set to automatically run on an git hook to validate commits as they are created locally. - validate-range: Run by CI for every PR, testing that all of the commits added by the PR are valid when considered together. Ensuring that all fixups are matched to another commit in the change. PR Close #36172
This commit is contained in:
parent
e893c5a330
commit
f40d51733a
|
@ -278,9 +278,10 @@ jobs:
|
|||
- run: 'yarn bazel:lint ||
|
||||
(echo -e "\n.bzl files have lint errors. Please run ''yarn bazel:lint-fix''"; exit 1)'
|
||||
|
||||
- run: yarn lint --branch $CI_GIT_BASE_REVISION
|
||||
- run: yarn ts-circular-deps:check
|
||||
- run: yarn -s lint --branch $CI_GIT_BASE_REVISION
|
||||
- run: yarn -s ts-circular-deps:check
|
||||
- run: yarn -s ng-dev pullapprove verify
|
||||
- run: yarn -s ng-dev commit-message validate-range --range $CI_COMMIT_RANGE
|
||||
|
||||
test:
|
||||
executor:
|
||||
|
|
|
@ -981,7 +981,6 @@ groups:
|
|||
'tools/ts-api-guardian/**',
|
||||
'tools/tslint/**',
|
||||
'tools/utils/**',
|
||||
'tools/validate-commit-message/**',
|
||||
'tools/yarn/**',
|
||||
'tools/*',
|
||||
'**/*.bzl',
|
||||
|
|
|
@ -8,13 +8,16 @@ ts_library(
|
|||
"config.ts",
|
||||
"validate.ts",
|
||||
"validate-file.ts",
|
||||
"validate-range.ts",
|
||||
],
|
||||
module_name = "@angular/dev-infra-private/commit-message",
|
||||
visibility = ["//dev-infra:__subpackages__"],
|
||||
deps = [
|
||||
"//dev-infra/utils:config",
|
||||
"@npm//@types/node",
|
||||
"@npm//@types/shelljs",
|
||||
"@npm//@types/yargs",
|
||||
"@npm//shelljs",
|
||||
"@npm//tslib",
|
||||
"@npm//yargs",
|
||||
],
|
||||
|
|
|
@ -7,12 +7,37 @@
|
|||
*/
|
||||
import * as yargs from 'yargs';
|
||||
import {validateFile} from './validate-file';
|
||||
import {validateCommitRange} from './validate-range';
|
||||
|
||||
/** Build the parser for the commit-message commands. */
|
||||
export function buildCommitMessageParser(localYargs: yargs.Argv) {
|
||||
return localYargs.help().strict().command(
|
||||
'pre-commit-validate', 'Validate the most recent commit message', {}, () => {
|
||||
return localYargs.help()
|
||||
.strict()
|
||||
.command(
|
||||
'pre-commit-validate', 'Validate the most recent commit message', {},
|
||||
() => {
|
||||
validateFile('.git/COMMIT_EDITMSG');
|
||||
})
|
||||
.command(
|
||||
'validate-range', 'Validate a range of commit messages', {
|
||||
'range': {
|
||||
description: 'The range of commits to check, e.g. --range abc123..xyz456',
|
||||
demandOption: ' A range must be provided, e.g. --range abc123..xyz456',
|
||||
type: 'string',
|
||||
requiresArg: true,
|
||||
},
|
||||
},
|
||||
argv => {
|
||||
// If on CI, and not pull request number is provided, assume the branch
|
||||
// being run on is an upstream branch.
|
||||
if (process.env['CI'] && process.env['CI_PULL_REQUEST'] === 'false') {
|
||||
console.info(
|
||||
`Since valid commit messages are enforced by PR linting on CI, we do not\n` +
|
||||
`need to validate commit messages on CI runs on upstream branches.\n\n` +
|
||||
`Skipping check of provided commit range`);
|
||||
return;
|
||||
}
|
||||
validateCommitRange(argv.range);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
@ -17,5 +17,8 @@ export function validateFile(filePath: string) {
|
|||
const commitMessage = readFileSync(join(getRepoBaseDir(), filePath), 'utf8');
|
||||
if (validateCommitMessage(commitMessage)) {
|
||||
console.info('√ Valid commit message');
|
||||
return;
|
||||
}
|
||||
// If the validation did not return true, exit as a failure.
|
||||
process.exit(1);
|
||||
}
|
||||
|
|
|
@ -0,0 +1,45 @@
|
|||
/**
|
||||
* @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
|
||||
*/
|
||||
import {exec} from 'shelljs';
|
||||
import {parseCommitMessage, validateCommitMessage, ValidateCommitMessageOptions} from './validate';
|
||||
|
||||
// Whether the provided commit is a fixup commit.
|
||||
const isNonFixup = (m: string) => !parseCommitMessage(m).isFixup;
|
||||
|
||||
/** Validate all commits in a provided git commit range. */
|
||||
export function validateCommitRange(range: string) {
|
||||
// A random value is used as a string to allow for a definite split point in the git log result.
|
||||
const randomValueSeparator = `${Math.random()}`;
|
||||
// Custom git log format that provides the commit header and body, separated as expected with
|
||||
// the custom separator as the trailing value.
|
||||
const gitLogFormat = `%s%n%n%b${randomValueSeparator}`;
|
||||
|
||||
// Retrieve the commits in the provided range.
|
||||
const result = exec(`git log --reverse --format=${gitLogFormat} ${range}`, {silent: true});
|
||||
if (result.code) {
|
||||
throw new Error(`Failed to get all commits in the range: \n ${result.stderr}`);
|
||||
}
|
||||
|
||||
// Separate the commits from a single string into individual commits
|
||||
const commits = result.split(randomValueSeparator).map(l => l.trim()).filter(line => !!line);
|
||||
|
||||
console.info(`Examining ${commits.length} commit(s) in the provided range: ${range}`);
|
||||
|
||||
// Check each commit in the commit range. Commits are allowed to be fixup commits for other
|
||||
// commits in the provided commit range.
|
||||
const allCommitsInRangeValid = commits.every((m, i) => {
|
||||
const options: ValidateCommitMessageOptions = {
|
||||
disallowSquash: true,
|
||||
nonFixupCommitHeaders: isNonFixup(m) ? undefined : commits.slice(0, i).filter(isNonFixup)
|
||||
};
|
||||
return validateCommitMessage(m, options);
|
||||
});
|
||||
if (allCommitsInRangeValid) {
|
||||
console.info('√ All commit messages in range valid.');
|
||||
}
|
||||
}
|
|
@ -50,7 +50,6 @@ describe('validate-commit-message.js', () => {
|
|||
});
|
||||
|
||||
describe('validateMessage()', () => {
|
||||
|
||||
it('should be valid', () => {
|
||||
expect(validateCommitMessage('feat(packaging): something')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
@ -66,7 +65,6 @@ describe('validate-commit-message.js', () => {
|
|||
|
||||
expect(validateCommitMessage('Revert: "release(packaging): something"')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
});
|
||||
|
||||
it('should validate max length', () => {
|
||||
|
@ -74,8 +72,8 @@ describe('validate-commit-message.js', () => {
|
|||
'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
`The commit message header is longer than ${config.commitMessage.maxLineLength} characters`);
|
||||
expect(lastError).toContain(`The commit message header is longer than ${
|
||||
config.commitMessage.maxLineLength} characters`);
|
||||
});
|
||||
|
||||
it('should validate "<type>(<scope>): <subject>" format', () => {
|
||||
|
@ -130,7 +128,6 @@ describe('validate-commit-message.js', () => {
|
|||
});
|
||||
|
||||
describe('(revert)', () => {
|
||||
|
||||
it('should allow valid "revert: ..." syntaxes', () => {
|
||||
expect(validateCommitMessage('revert: anything')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
@ -143,7 +140,6 @@ describe('validate-commit-message.js', () => {
|
|||
|
||||
expect(validateCommitMessage('rEvErT anything')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
});
|
||||
|
||||
it('should not allow "revert(scope): ..." syntax', () => {
|
||||
|
@ -164,31 +160,29 @@ describe('validate-commit-message.js', () => {
|
|||
});
|
||||
|
||||
describe('(squash)', () => {
|
||||
|
||||
it('should strip the `squash! ` prefix and validate the rest', () => {
|
||||
const errorMessage = `The commit message header does not match the expected format.`;
|
||||
|
||||
// Valid messages.
|
||||
expect(validateCommitMessage('squash! feat(core): add feature')).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix: a bug', false)).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix: a bug', {disallowSquash: false})).toBe(VALID);
|
||||
|
||||
// Invalid messages.
|
||||
expect(validateCommitMessage('squash! fix a typo', false)).toBe(INVALID);
|
||||
expect(validateCommitMessage('squash! fix a typo', {disallowSquash: false})).toBe(INVALID);
|
||||
expect(lastError).toContain('squash! fix a typo');
|
||||
expect(lastError).toContain(errorMessage);
|
||||
|
||||
expect(validateCommitMessage('squash! squash! fix: a bug')).toBe(INVALID);
|
||||
expect(lastError).toContain('squash! squash! fix: a bug');
|
||||
expect(lastError).toContain(errorMessage);
|
||||
|
||||
|
||||
});
|
||||
|
||||
describe('with `disallowSquash`', () => {
|
||||
|
||||
it('should fail', () => {
|
||||
expect(validateCommitMessage('fix(core): something', true)).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix(core): something', true)).toBe(INVALID);
|
||||
expect(validateCommitMessage('fix(core): something', {disallowSquash: true})).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix(core): something', {
|
||||
disallowSquash: true
|
||||
})).toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'The commit must be manually squashed into the target commit');
|
||||
});
|
||||
|
@ -196,9 +190,7 @@ describe('validate-commit-message.js', () => {
|
|||
});
|
||||
|
||||
describe('(fixup)', () => {
|
||||
|
||||
describe('without `nonFixupCommitHeaders`', () => {
|
||||
|
||||
it('should strip the `fixup! ` prefix and validate the rest', () => {
|
||||
const errorMessage = `The commit message header does not match the expected format.`;
|
||||
|
||||
|
@ -214,21 +206,26 @@ describe('validate-commit-message.js', () => {
|
|||
expect(validateCommitMessage('fixup! fixup! fix: a bug')).toBe(INVALID);
|
||||
expect(lastError).toContain('fixup! fixup! fix: a bug');
|
||||
expect(lastError).toContain(errorMessage);
|
||||
|
||||
|
||||
});
|
||||
});
|
||||
|
||||
describe('with `nonFixupCommitHeaders`', () => {
|
||||
|
||||
it('should check that the fixup commit matches a non-fixup one', () => {
|
||||
const msg = 'fixup! foo';
|
||||
|
||||
expect(validateCommitMessage(msg, false, ['foo', 'bar', 'baz'])).toBe(VALID);
|
||||
expect(validateCommitMessage(msg, false, ['bar', 'baz', 'foo'])).toBe(VALID);
|
||||
expect(validateCommitMessage(msg, false, ['baz', 'foo', 'bar'])).toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['foo', 'bar', 'baz']}))
|
||||
.toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['bar', 'baz', 'foo']}))
|
||||
.toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['baz', 'foo', 'bar']}))
|
||||
.toBe(VALID);
|
||||
|
||||
expect(validateCommitMessage(msg, false, ['qux', 'quux', 'quuux'])).toBe(INVALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['qux', 'quux', 'quuux']}))
|
||||
.toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'Unable to find match for fixup commit among prior commits: \n' +
|
||||
' qux\n' +
|
||||
|
@ -237,8 +234,13 @@ describe('validate-commit-message.js', () => {
|
|||
});
|
||||
|
||||
it('should fail if `nonFixupCommitHeaders` is empty', () => {
|
||||
expect(validateCommitMessage('refactor(core): make reactive', false, [])).toBe(VALID);
|
||||
expect(validateCommitMessage('fixup! foo', false, [])).toBe(INVALID);
|
||||
expect(validateCommitMessage('refactor(core): make reactive', {
|
||||
disallowSquash: false,
|
||||
nonFixupCommitHeaders: []
|
||||
})).toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
'fixup! foo', {disallowSquash: false, nonFixupCommitHeaders: []}))
|
||||
.toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
`Unable to find match for fixup commit among prior commits: -`);
|
||||
});
|
||||
|
|
|
@ -8,6 +8,12 @@
|
|||
import {getAngularDevConfig} from '../utils/config';
|
||||
import {CommitMessageConfig} from './config';
|
||||
|
||||
/** Options for commit message validation. */
|
||||
export interface ValidateCommitMessageOptions {
|
||||
disallowSquash?: boolean;
|
||||
nonFixupCommitHeaders?: string[];
|
||||
}
|
||||
|
||||
const FIXUP_PREFIX_RE = /^fixup! /i;
|
||||
const GITHUB_LINKING_RE = /((closed?s?)|(fix(es)?(ed)?)|(resolved?s?))\s\#(\d+)/ig;
|
||||
const SQUASH_PREFIX_RE = /^squash! /i;
|
||||
|
@ -54,10 +60,9 @@ export function parseCommitMessage(commitMsg: string) {
|
|||
};
|
||||
}
|
||||
|
||||
|
||||
/** Validate a commit message against using the local repo's config. */
|
||||
export function validateCommitMessage(
|
||||
commitMsg: string, disallowSquash: boolean = false, nonFixupCommitHeaders?: string[]) {
|
||||
commitMsg: string, options: ValidateCommitMessageOptions = {}) {
|
||||
function error(errorMessage: string) {
|
||||
console.error(
|
||||
`INVALID COMMIT MSG: \n` +
|
||||
|
@ -78,7 +83,7 @@ export function validateCommitMessage(
|
|||
return true;
|
||||
}
|
||||
|
||||
if (commit.isSquash && disallowSquash) {
|
||||
if (commit.isSquash && options.disallowSquash) {
|
||||
error('The commit must be manually squashed into the target commit');
|
||||
return false;
|
||||
}
|
||||
|
@ -86,11 +91,11 @@ export function validateCommitMessage(
|
|||
// If it is a fixup commit and `nonFixupCommitHeaders` is not empty, we only care to check whether
|
||||
// there is a corresponding non-fixup commit (i.e. a commit whose header is identical to this
|
||||
// commit's header after stripping the `fixup! ` prefix).
|
||||
if (commit.isFixup && nonFixupCommitHeaders) {
|
||||
if (!nonFixupCommitHeaders.includes(commit.header)) {
|
||||
if (commit.isFixup && options.nonFixupCommitHeaders) {
|
||||
if (!options.nonFixupCommitHeaders.includes(commit.header)) {
|
||||
error(
|
||||
'Unable to find match for fixup commit among prior commits: ' +
|
||||
(nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-'));
|
||||
(options.nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-'));
|
||||
return false;
|
||||
}
|
||||
|
||||
|
@ -118,8 +123,8 @@ export function validateCommitMessage(
|
|||
}
|
||||
|
||||
if (commit.bodyWithoutLinking.trim().length < config.minBodyLength) {
|
||||
error(
|
||||
`The commit message body does not meet the minimum length of ${config.minBodyLength} characters`);
|
||||
error(`The commit message body does not meet the minimum length of ${
|
||||
config.minBodyLength} characters`);
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
|
@ -49,8 +49,7 @@ gulp.task('format:changed:enforce', ['format:untracked:enforce', 'format:diff:en
|
|||
// Alias for `format:changed` that formerly formatted all files.
|
||||
gulp.task('format', ['format:changed']);
|
||||
|
||||
gulp.task('lint', ['format:changed:enforce', 'validate-commit-messages']);
|
||||
gulp.task('validate-commit-messages', loadTask('validate-commit-message'));
|
||||
gulp.task('lint', ['format:changed:enforce']);
|
||||
gulp.task('source-map-test', loadTask('source-map-test'));
|
||||
gulp.task('changelog', loadTask('changelog'));
|
||||
gulp.task('changelog:zonejs', loadTask('changelog-zonejs'));
|
||||
|
|
|
@ -22,7 +22,7 @@
|
|||
"preinstall": "node tools/yarn/check-yarn.js",
|
||||
"postinstall": "node scripts/webdriver-manager-update.js && node --preserve-symlinks --preserve-symlinks-main ./tools/postinstall-patches.js",
|
||||
"check-env": "gulp check-env",
|
||||
"commitmsg": "node ./scripts/git/commit-msg.js",
|
||||
"commitmsg": "yarn -s ng-dev commit-message pre-commit-validate",
|
||||
"test-ivy-aot": "bazelisk test --config=ivy --build_tag_filters=-no-ivy-aot,-fixme-ivy-aot --test_tag_filters=-no-ivy-aot,-fixme-ivy-aot",
|
||||
"test-non-ivy": "bazelisk test --build_tag_filters=-ivy-only --test_tag_filters=-ivy-only",
|
||||
"test-fixme-ivy-aot": "bazelisk test --config=ivy --build_tag_filters=-no-ivy-aot --test_tag_filters=-no-ivy-aot",
|
||||
|
|
|
@ -1,34 +0,0 @@
|
|||
#! /usr/bin/env node
|
||||
/**
|
||||
* @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
|
||||
*/
|
||||
|
||||
// git commit-msg hook to check the commit message against Angular conventions
|
||||
// see `/CONTRIBUTING.md` for mode details.
|
||||
|
||||
'use strict';
|
||||
|
||||
const fs = require('fs');
|
||||
const checkMsg = require('../../tools/validate-commit-message');
|
||||
const msgFile = process.env['GIT_PARAMS'];
|
||||
|
||||
let isValid = true;
|
||||
|
||||
if (msgFile) {
|
||||
const commitMsg = fs.readFileSync(msgFile, {encoding: 'utf-8'});
|
||||
const firstLine = commitMsg.split('\n')[0];
|
||||
isValid = checkMsg(firstLine);
|
||||
|
||||
if (!isValid) {
|
||||
console.error(
|
||||
'\nCheck CONTRIBUTING.md at the root of the repo for more information.' +
|
||||
'\n' +
|
||||
'\n(In case you need the invalid commit message, it should be stored in \'.git/COMMIT_EDITMSG\'.)');
|
||||
}
|
||||
}
|
||||
|
||||
process.exit(isValid ? 0 : 1);
|
|
@ -1,92 +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-console
|
||||
module.exports = (gulp) => async() => {
|
||||
try {
|
||||
const validateCommitMessage = require('../validate-commit-message');
|
||||
const shelljs = require('shelljs');
|
||||
const getRefsAndShasForTarget = require('../utils/get-refs-and-shas-for-target');
|
||||
|
||||
// Break on error.
|
||||
shelljs.set('-e');
|
||||
|
||||
|
||||
let target = {};
|
||||
if (process.env['CI'] === 'true') {
|
||||
// Validation of commit messages on CI
|
||||
if (process.env['CI_PULL_REQUEST'] === 'false') {
|
||||
// Skip commit message validation on CI for non-PR branches as we are not testing new
|
||||
// unreviewed commits. By enforcing correctness on the incoming changes in the PR
|
||||
// branches, we are able to render this check unnecessary on non-PR branches.
|
||||
console.info(
|
||||
`Since valid commit messages are enforced by PR linting on CI,\n` +
|
||||
`we do not need to validate commit messages on CI runs on upstream branches.\n\n` +
|
||||
`Skipping validate-commit-message check`);
|
||||
process.exit();
|
||||
}
|
||||
target = await getRefsAndShasForTarget(process.env['CI_PULL_REQUEST']);
|
||||
} else {
|
||||
// Validation of commit messages locally
|
||||
const baseRef = 'master';
|
||||
const headRef = shelljs.exec('git symbolic-ref --short HEAD', {silent: true}).trim();
|
||||
const baseSha = shelljs.exec(`git rev-parse origin/master`, {silent: true}).trim();
|
||||
const headSha = shelljs.exec(`git rev-parse HEAD`, {silent: true}).trim();
|
||||
const commonAncestorSha =
|
||||
shelljs.exec(`git merge-base origin/master ${headSha}`, {silent: true}).trim();
|
||||
target = {
|
||||
base: {
|
||||
ref: baseRef,
|
||||
sha: baseSha,
|
||||
},
|
||||
head: {
|
||||
ref: headRef,
|
||||
sha: headSha,
|
||||
},
|
||||
commonAncestorSha: commonAncestorSha,
|
||||
latestShaOfTargetBranch: baseSha,
|
||||
latestShaOfPrBranch: headSha,
|
||||
};
|
||||
}
|
||||
|
||||
const result = shelljs.exec(
|
||||
`git log --reverse --format=%s ${target.commonAncestorSha}..${target.latestShaOfPrBranch}`,
|
||||
{silent: true});
|
||||
|
||||
if (result.code) {
|
||||
throw new Error(`Failed to fetch commits: ${result.stderr}`);
|
||||
}
|
||||
|
||||
const commitsByLine = result.trim().split(/\n/).filter(line => line != '');
|
||||
|
||||
console.log(`Examining ${commitsByLine.length} commit(s) between ${target.base.ref} and HEAD`);
|
||||
|
||||
if (commitsByLine.length == 0) {
|
||||
console.log(`There are zero new commits between ${target.base.ref} and HEAD`);
|
||||
}
|
||||
|
||||
const disallowSquashCommits = true;
|
||||
const isNonFixup = m => !validateCommitMessage.FIXUP_PREFIX_RE.test(m);
|
||||
const someCommitsInvalid = !commitsByLine.every((m, i) => {
|
||||
// `priorNonFixupCommits` is only needed if the current commit is a fixup commit.
|
||||
const priorNonFixupCommits =
|
||||
isNonFixup(m) ? undefined : commitsByLine.slice(0, i).filter(isNonFixup);
|
||||
return validateCommitMessage(m, disallowSquashCommits, priorNonFixupCommits);
|
||||
});
|
||||
|
||||
if (someCommitsInvalid) {
|
||||
throw new Error(
|
||||
'Please fix the failing commit messages before continuing...\n' +
|
||||
'Commit message guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines');
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(err);
|
||||
process.exit(1);
|
||||
}
|
||||
};
|
|
@ -1,9 +0,0 @@
|
|||
load("//tools:defaults.bzl", "jasmine_node_test")
|
||||
|
||||
jasmine_node_test(
|
||||
name = "validate-commit-message",
|
||||
srcs = glob(["*.js"]),
|
||||
data = [
|
||||
"commit-message.json",
|
||||
],
|
||||
)
|
|
@ -1,44 +0,0 @@
|
|||
{
|
||||
"maxLength": 120,
|
||||
"types": [
|
||||
"build",
|
||||
"ci",
|
||||
"docs",
|
||||
"feat",
|
||||
"fix",
|
||||
"perf",
|
||||
"refactor",
|
||||
"release",
|
||||
"style",
|
||||
"test"
|
||||
],
|
||||
"scopes": [
|
||||
"animations",
|
||||
"bazel",
|
||||
"benchpress",
|
||||
"changelog",
|
||||
"common",
|
||||
"compiler",
|
||||
"compiler-cli",
|
||||
"core",
|
||||
"dev-infra",
|
||||
"docs-infra",
|
||||
"elements",
|
||||
"forms",
|
||||
"http",
|
||||
"language-service",
|
||||
"localize",
|
||||
"ngcc",
|
||||
"packaging",
|
||||
"platform-browser",
|
||||
"platform-browser-dynamic",
|
||||
"platform-server",
|
||||
"platform-webworker",
|
||||
"platform-webworker-dynamic",
|
||||
"router",
|
||||
"service-worker",
|
||||
"upgrade",
|
||||
"ve",
|
||||
"zone.js"
|
||||
]
|
||||
}
|
|
@ -1,9 +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
|
||||
*/
|
||||
|
||||
module.exports = require('./validate-commit-message');
|
|
@ -1,100 +0,0 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
/**
|
||||
* @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
|
||||
*/
|
||||
|
||||
/**
|
||||
* GIT commit message format enforcement
|
||||
*
|
||||
* Note: this script was originally written by Vojta for AngularJS :-)
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const config = require('./commit-message.json');
|
||||
const FIXUP_PREFIX_RE = /^fixup! /i;
|
||||
const SQUASH_PREFIX_RE = /^squash! /i;
|
||||
const REVERT_PREFIX_RE = /^revert:? /i;
|
||||
|
||||
module.exports = (commitHeader, disallowSquash, nonFixupCommitHeaders) => {
|
||||
if (REVERT_PREFIX_RE.test(commitHeader)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const {header, type, scope, isFixup, isSquash} = parseCommitHeader(commitHeader);
|
||||
|
||||
if (isSquash && disallowSquash) {
|
||||
error('The commit must be manually squashed into the target commit', commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
// If it is a fixup commit and `nonFixupCommitHeaders` is not empty, we only care to check whether
|
||||
// there is a corresponding non-fixup commit (i.e. a commit whose header is identical to this
|
||||
// commit's header after stripping the `fixup! ` prefix).
|
||||
if (isFixup && nonFixupCommitHeaders) {
|
||||
if (!nonFixupCommitHeaders.includes(header)) {
|
||||
error(
|
||||
'Unable to find match for fixup commit among prior commits: ' +
|
||||
(nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-'),
|
||||
commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
if (header.length > config.maxLength) {
|
||||
error(`The commit message header is longer than ${config.maxLength} characters`, commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!type) {
|
||||
const format = '<type>(<scope>): <subject>';
|
||||
error(
|
||||
`The commit message header does not match the format of '${format}' or 'Revert: "${format}"'`,
|
||||
commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!config.types.includes(type)) {
|
||||
error(`'${type}' is not an allowed type.\n => TYPES: ${config.types.join(', ')}`, commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (scope && !config.scopes.includes(scope)) {
|
||||
error(
|
||||
`'${scope}' is not an allowed scope.\n => SCOPES: ${config.scopes.join(', ')}`,
|
||||
commitHeader);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
};
|
||||
|
||||
module.exports.FIXUP_PREFIX_RE = FIXUP_PREFIX_RE;
|
||||
module.exports.config = config;
|
||||
|
||||
// Helpers
|
||||
function error(errorMessage, commitHeader) {
|
||||
console.error(`INVALID COMMIT MSG: ${commitHeader}\n => ERROR: ${errorMessage}`);
|
||||
}
|
||||
|
||||
function parseCommitHeader(header) {
|
||||
const isFixup = FIXUP_PREFIX_RE.test(header);
|
||||
const isSquash = SQUASH_PREFIX_RE.test(header);
|
||||
header = header.replace(FIXUP_PREFIX_RE, '').replace(SQUASH_PREFIX_RE, '');
|
||||
|
||||
const match = /^(\w+)(?:\(([^)]+)\))?\: (.+)$/.exec(header) || [];
|
||||
|
||||
return {
|
||||
header,
|
||||
type: match[1],
|
||||
scope: match[2],
|
||||
subject: match[3], isFixup, isSquash,
|
||||
};
|
||||
}
|
|
@ -1,234 +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
|
||||
*/
|
||||
|
||||
// Imports
|
||||
const validateMessage = require('./validate-commit-message');
|
||||
|
||||
// Constants
|
||||
const TYPES = validateMessage.config.types.join(', ');
|
||||
const SCOPES = validateMessage.config.scopes.join(', ');
|
||||
|
||||
const INVALID = false;
|
||||
const VALID = true;
|
||||
|
||||
|
||||
describe('validate-commit-message.js', () => {
|
||||
let errors = [];
|
||||
let logs = [];
|
||||
|
||||
// Helpers
|
||||
const stripColor = msg => msg.replace(/\x1B\[\d+m/g, '');
|
||||
|
||||
beforeEach(() => {
|
||||
errors = [];
|
||||
logs = [];
|
||||
|
||||
spyOn(console, 'error').and.callFake(msg => errors.push(stripColor(msg)));
|
||||
spyOn(console, 'log').and.callFake(msg => logs.push(stripColor(msg)));
|
||||
});
|
||||
|
||||
describe('validateMessage()', () => {
|
||||
|
||||
it('should be valid', () => {
|
||||
expect(validateMessage('fix(core): something')).toBe(VALID);
|
||||
expect(validateMessage('feat(common): something')).toBe(VALID);
|
||||
expect(validateMessage('docs(compiler): something')).toBe(VALID);
|
||||
expect(validateMessage('style(http): something')).toBe(VALID);
|
||||
expect(validateMessage('refactor(platform-webworker): something')).toBe(VALID);
|
||||
expect(validateMessage('test(language-service): something')).toBe(VALID);
|
||||
expect(validateMessage('test(packaging): something')).toBe(VALID);
|
||||
expect(validateMessage('release: something')).toBe(VALID);
|
||||
expect(validateMessage('release(packaging): something')).toBe(VALID);
|
||||
expect(validateMessage('release(packaging): something')).toBe(VALID);
|
||||
expect(validateMessage('fixup! release(packaging): something')).toBe(VALID);
|
||||
expect(validateMessage('squash! release(packaging): something')).toBe(VALID);
|
||||
expect(validateMessage('Revert: "release(packaging): something"')).toBe(VALID);
|
||||
expect(validateMessage('Revert "release(packaging): something"')).toBe(VALID);
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('should validate max length', () => {
|
||||
var msg =
|
||||
'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...';
|
||||
|
||||
expect(validateMessage(msg)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header is longer than 120 characters`
|
||||
]);
|
||||
});
|
||||
|
||||
it('should validate "<type>(<scope>): <subject>" format', () => {
|
||||
const msg = 'not correct format';
|
||||
|
||||
expect(validateMessage(msg)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header does not match the format of '<type>(<scope>): <subject>' or 'Revert: "<type>(<scope>): <subject>"'`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should fail when type is invalid', () => {
|
||||
const msg = 'weird(common): something';
|
||||
|
||||
expect(validateMessage(msg)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n => ERROR: 'weird' is not an allowed type.\n => TYPES: ${TYPES}`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should fail when scope is invalid', () => {
|
||||
const errorMessageFor = (scope, header) =>
|
||||
`INVALID COMMIT MSG: ${header}\n => ERROR: '${scope}' is not an allowed scope.\n => SCOPES: ${SCOPES}`;
|
||||
|
||||
expect(validateMessage('fix(Compiler): something')).toBe(INVALID);
|
||||
expect(validateMessage('feat(bah): something')).toBe(INVALID);
|
||||
expect(validateMessage('style(webworker): something')).toBe(INVALID);
|
||||
expect(validateMessage('refactor(security): something')).toBe(INVALID);
|
||||
expect(validateMessage('refactor(docs): something')).toBe(INVALID);
|
||||
expect(validateMessage('release(angular): something')).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
errorMessageFor('Compiler', 'fix(Compiler): something'),
|
||||
errorMessageFor('bah', 'feat(bah): something'),
|
||||
errorMessageFor('webworker', 'style(webworker): something'),
|
||||
errorMessageFor('security', 'refactor(security): something'),
|
||||
errorMessageFor('docs', 'refactor(docs): something'),
|
||||
errorMessageFor('angular', 'release(angular): something'),
|
||||
]);
|
||||
});
|
||||
|
||||
it('should allow empty scope', () => {
|
||||
expect(validateMessage('fix: blablabla')).toBe(VALID);
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
// We do not want to allow WIP. It is OK to fail the PR build in this case to show that there is
|
||||
// work still to be done (i.e. fixing the commit message).
|
||||
it('should not allow "WIP: ..." syntax', () => {
|
||||
const msg = 'WIP: fix: something';
|
||||
|
||||
expect(validateMessage(msg)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n => ERROR: 'WIP' is not an allowed type.\n => TYPES: ${TYPES}`,
|
||||
]);
|
||||
});
|
||||
|
||||
describe('(revert)', () => {
|
||||
|
||||
it('should allow valid "revert: ..." syntaxes', () => {
|
||||
expect(validateMessage('revert: anything')).toBe(VALID);
|
||||
expect(validateMessage('Revert: "anything"')).toBe(VALID);
|
||||
expect(validateMessage('revert anything')).toBe(VALID);
|
||||
expect(validateMessage('rEvErT anything')).toBe(VALID);
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('should not allow "revert(scope): ..." syntax', () => {
|
||||
const msg = 'revert(compiler): reduce generated code payload size by 65%';
|
||||
|
||||
expect(validateMessage(msg)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n => ERROR: 'revert' is not an allowed type.\n => TYPES: ${TYPES}`,
|
||||
]);
|
||||
});
|
||||
|
||||
// https://github.com/angular/angular/issues/23479
|
||||
it('should allow typical Angular messages generated by git', () => {
|
||||
const msg =
|
||||
'Revert "fix(compiler): Pretty print object instead of [Object object] (#22689)" (#23442)';
|
||||
|
||||
expect(validateMessage(msg)).toBe(VALID);
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('(squash)', () => {
|
||||
|
||||
it('should strip the `squash! ` prefix and validate the rest', () => {
|
||||
const errorMessageFor = header =>
|
||||
`INVALID COMMIT MSG: ${header}\n => ERROR: The commit message header does not match the format of ` +
|
||||
'\'<type>(<scope>): <subject>\' or \'Revert: "<type>(<scope>): <subject>"\'';
|
||||
|
||||
// Valid messages.
|
||||
expect(validateMessage('squash! feat(core): add feature')).toBe(VALID);
|
||||
expect(validateMessage('squash! fix: a bug', false)).toBe(VALID);
|
||||
|
||||
// Invalid messages.
|
||||
expect(validateMessage('squash! fix a typo', false)).toBe(INVALID);
|
||||
expect(validateMessage('squash! squash! fix: a bug')).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
errorMessageFor('squash! fix a typo'),
|
||||
errorMessageFor('squash! squash! fix: a bug'),
|
||||
]);
|
||||
});
|
||||
|
||||
describe('with `disallowSquash`', () => {
|
||||
|
||||
it('should fail', () => {
|
||||
expect(validateMessage('fix: something', true)).toBe(VALID);
|
||||
expect(validateMessage('squash! fix: something', true)).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
'INVALID COMMIT MSG: squash! fix: something\n' +
|
||||
' => ERROR: The commit must be manually squashed into the target commit',
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('(fixup)', () => {
|
||||
|
||||
describe('without `nonFixupCommitHeaders`', () => {
|
||||
|
||||
it('should strip the `fixup! ` prefix and validate the rest', () => {
|
||||
const errorMessageFor = header =>
|
||||
`INVALID COMMIT MSG: ${header}\n => ERROR: The commit message header does not match the format of ` +
|
||||
'\'<type>(<scope>): <subject>\' or \'Revert: "<type>(<scope>): <subject>"\'';
|
||||
|
||||
// Valid messages.
|
||||
expect(validateMessage('fixup! feat(core): add feature')).toBe(VALID);
|
||||
expect(validateMessage('fixup! fix: a bug')).toBe(VALID);
|
||||
|
||||
// Invalid messages.
|
||||
expect(validateMessage('fixup! fix a typo')).toBe(INVALID);
|
||||
expect(validateMessage('fixup! fixup! fix: a bug')).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
errorMessageFor('fixup! fix a typo'),
|
||||
errorMessageFor('fixup! fixup! fix: a bug'),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('with `nonFixupCommitHeaders`', () => {
|
||||
|
||||
it('should check that the fixup commit matches a non-fixup one', () => {
|
||||
const msg = 'fixup! foo';
|
||||
|
||||
expect(validateMessage(msg, false, ['foo', 'bar', 'baz'])).toBe(VALID);
|
||||
expect(validateMessage(msg, false, ['bar', 'baz', 'foo'])).toBe(VALID);
|
||||
expect(validateMessage(msg, false, ['baz', 'foo', 'bar'])).toBe(VALID);
|
||||
|
||||
expect(validateMessage(msg, false, ['qux', 'quux', 'quuux'])).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
`INVALID COMMIT MSG: ${msg}\n` +
|
||||
' => ERROR: Unable to find match for fixup commit among prior commits: \n' +
|
||||
' qux\n' +
|
||||
' quux\n' +
|
||||
' quuux',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should fail if `nonFixupCommitHeaders` is empty', () => {
|
||||
expect(validateMessage('refactor(router): make reactive', false, [])).toBe(VALID);
|
||||
expect(validateMessage('fixup! foo', false, [])).toBe(INVALID);
|
||||
expect(errors).toEqual([
|
||||
'INVALID COMMIT MSG: fixup! foo\n' +
|
||||
' => ERROR: Unable to find match for fixup commit among prior commits: -',
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
Loading…
Reference in New Issue