Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ci: validate the message of each new commit as part of the CI linting
This patch adds the gulp command of `validate-commit-messages` which will validate the range of commits messages present in the active branch. This check now runs on CI as part of the linting checks. Allowed commit message types and scopes are controlled via commit-message.json file and documented at https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines This solution is based on old Vojta's code that he wrote for angular/angular.js, that was later adjusted by @matsko in #13815. Ideally we should switch over to something like https://www.npmjs.com/package/commitplease as suggested in #9953 but that package currently doesn't support strict scope checking, which is one of the primarily goal of this PR. Note that this PR removes support for "chore" which was previously overused by everyone on the team. Closes #13815 Fixes #3337
- Loading branch information
Showing
8 changed files
with
290 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"maxLength": 100, | ||
"types": [ | ||
"build", | ||
"ci", | ||
"docs", | ||
"feat", | ||
"fix", | ||
"perf", | ||
"refactor", | ||
"style", | ||
"test" | ||
], | ||
"scopes": [ | ||
"benchpress", | ||
"common", | ||
"compiler", | ||
"compiler-cli", | ||
"core", | ||
"forms", | ||
"http", | ||
"language-service", | ||
"platform-browser", | ||
"platform-browser-dynamic", | ||
"platform-server", | ||
"platform-webworker", | ||
"platform-webworker-dynamic", | ||
"router", | ||
"upgrade", | ||
"tsc-wrapped", | ||
|
||
"packaging", | ||
"changelog" | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require('./validate-commit-message'); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"spec_dir": "", | ||
"spec_files": [ | ||
"**/*[sS]pec.js" | ||
], | ||
"helpers": [ | ||
"helpers/**/*.js" | ||
], | ||
"stopSpecOnExpectationFailure": false, | ||
"random": false | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
#!/usr/bin/env node | ||
|
||
/** | ||
* GIT commit message format enforcement | ||
* | ||
* Note: this script was originally written by Vojta for AngularJS :-) | ||
*/ | ||
|
||
'use strict'; | ||
|
||
const fs = require('fs'); | ||
const path = require('path'); | ||
const configPath = path.resolve(__dirname, './commit-message.json'); | ||
const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); | ||
const PATTERN = /^(revert\: )?(\w+)(?:\(([^)]+)\))?\: (.+)$/; | ||
|
||
module.exports = function(commitSubject) { | ||
if (commitSubject.length > config['maxLength']) { | ||
error(`The commit message is longer than ${config['maxLength']} characters`, commitSubject); | ||
return false; | ||
} | ||
|
||
const match = PATTERN.exec(commitSubject); | ||
if (!match || match[2] === 'revert') { | ||
error( | ||
`The commit message does not match the format of "<type>(<scope>): <subject> OR revert: type(<scope>): <subject>"`, | ||
commitSubject); | ||
return false; | ||
} | ||
|
||
const type = match[2]; | ||
if (config['types'].indexOf(type) === -1) { | ||
error( | ||
`${type} is not an allowed type.\n => TYPES: ${config['types'].join(', ')}`, commitSubject); | ||
return false; | ||
} | ||
|
||
const scope = match[3]; | ||
|
||
if (scope && !config['scopes'].includes(scope)) { | ||
error( | ||
`"${scope}" is not an allowed scope.\n => SCOPES: ${config['scopes'].join(', ')}`, | ||
commitSubject); | ||
return false; | ||
} | ||
|
||
return true; | ||
}; | ||
|
||
function error(errorMessage, commitMessage) { | ||
console.error(`INVALID COMMIT MSG: "${commitMessage}"\n => ERROR: ${errorMessage}`); | ||
} |
119 changes: 119 additions & 0 deletions
119
tools/validate-commit-message/validate-commit-message.spec.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
'use strict'; | ||
|
||
describe('validate-commit-message.js', function() { | ||
var validateMessage = require('./validate-commit-message'); | ||
var errors = []; | ||
var logs = []; | ||
|
||
var VALID = true; | ||
var INVALID = false; | ||
|
||
beforeEach(function() { | ||
errors.length = 0; | ||
logs.length = 0; | ||
|
||
spyOn(console, 'error').and.callFake(function(msg) { | ||
errors.push(msg.replace(/\x1B\[\d+m/g, '')); // uncolor | ||
}); | ||
|
||
spyOn(console, 'log').and.callFake(function(msg) { | ||
logs.push(msg.replace(/\x1B\[\d+m/g, '')); // uncolor | ||
}); | ||
}); | ||
|
||
describe('validateMessage', function() { | ||
|
||
it('should be valid', function() { | ||
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(errors).toEqual([]); | ||
}); | ||
|
||
|
||
it('should fail when scope is invalid', function() { | ||
expect(validateMessage('fix(Compiler): something')).toBe(INVALID); | ||
expect(validateMessage('feat(bah): something')).toBe(INVALID); | ||
expect(validateMessage('docs(animations): something')).toBe(INVALID); | ||
expect(validateMessage('style(webworker): something')).toBe(INVALID); | ||
expect(validateMessage('refactor(security): something')).toBe(INVALID); | ||
expect(validateMessage('refactor(docs): something')).toBe(INVALID); | ||
['INVALID COMMIT MSG: "fix(Compiler): something"\n' + | ||
' => ERROR: "Compiler" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog', | ||
'INVALID COMMIT MSG: "feat(bah): something"\n' + | ||
' => ERROR: "bah" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog', | ||
'INVALID COMMIT MSG: "docs(animations): something"\n' + | ||
' => ERROR: "animations" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog', | ||
'INVALID COMMIT MSG: "style(webworker): something"\n' + | ||
' => ERROR: "webworker" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog', | ||
'INVALID COMMIT MSG: "refactor(security): something"\n' + | ||
' => ERROR: "security" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog', | ||
'INVALID COMMIT MSG: "refactor(docs): something"\n' + | ||
' => ERROR: "docs" is not an allowed scope.\n' + | ||
' => SCOPES: benchpress, common, compiler, compiler-cli, core, forms, http, language-service, platform-browser, platform-browser-dynamic, platform-server, platform-webworker, platform-webworker-dynamic, router, upgrade, tsc-wrapped, packaging, changelog'] | ||
.forEach((expectedErrorMessage, index) => { | ||
expect(expectedErrorMessage).toEqual(errors[index]); | ||
}); | ||
}); | ||
|
||
|
||
it('should validate 100 characters length', function() { | ||
var msg = | ||
'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer... '; | ||
|
||
expect(validateMessage(msg)).toBe(INVALID); | ||
expect(errors).toEqual([ | ||
'INVALID COMMIT MSG: "fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer... "\n => ERROR: The commit message is longer than 100 characters' | ||
]); | ||
}); | ||
|
||
|
||
it('should validate "<type>(<scope>): <subject>" format', function() { | ||
var msg = 'not correct format'; | ||
|
||
expect(validateMessage(msg)).toBe(INVALID); | ||
expect(errors).toEqual([ | ||
'INVALID COMMIT MSG: "not correct format"\n => ERROR: The commit message does not match the format of "<type>(<scope>): <subject> OR revert: type(<scope>): <subject>"' | ||
]); | ||
}); | ||
|
||
|
||
it('should support "revert: type(scope):" syntax and reject "revert(scope):" syntax', function() { | ||
let correctMsg = 'revert: fix(compiler): reduce generated code payload size by 65%'; | ||
expect(validateMessage(correctMsg)).toBe(VALID); | ||
|
||
let incorretMsg = 'revert(compiler): reduce generated code payload size by 65%'; | ||
expect(validateMessage(incorretMsg)).toBe(INVALID); | ||
expect(errors).toEqual([ | ||
'INVALID COMMIT MSG: "revert(compiler): reduce generated code payload size by 65%"\n => ERROR: The commit message does not match the format of "<type>(<scope>): <subject> OR revert: type(<scope>): <subject>"' | ||
]); | ||
}); | ||
|
||
|
||
it('should validate type', function() { | ||
expect(validateMessage('weird($filter): something')).toBe(INVALID); | ||
expect(errors).toEqual( | ||
['INVALID COMMIT MSG: "weird($filter): something"\n' + | ||
' => ERROR: weird is not an allowed type.\n' + | ||
' => TYPES: build, ci, docs, feat, fix, perf, refactor, style, test']); | ||
}); | ||
|
||
|
||
it('should allow empty scope', | ||
function() { expect(validateMessage('fix: blablabla')).toBe(VALID); }); | ||
|
||
// we don't want to allow WIP. it's ok to fail the PR build in this case to show that there is | ||
// work still to be done. | ||
it('should not ignore msg prefixed with "WIP: "', | ||
function() { expect(validateMessage('WIP: bullshit')).toBe(INVALID); }); | ||
}); | ||
}); |