diff --git a/.dev-infra.json b/.dev-infra.json index 0a0997a8e9..2e1c7ddc39 100644 --- a/.dev-infra.json +++ b/.dev-infra.json @@ -1,7 +1,7 @@ { "commitMessage": { "maxLength": 120, - "minBodyLength": 0, + "minBodyLength": 100, "types": [ "build", "ci", @@ -44,4 +44,4 @@ "zone.js" ] } -} \ No newline at end of file +} diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts index 3029613e11..5017ea15f6 100644 --- a/dev-infra/commit-message/validate.spec.ts +++ b/dev-infra/commit-message/validate.spec.ts @@ -160,21 +160,12 @@ 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', {disallowSquash: false})).toBe(VALID); - - // Invalid messages. - 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('without `disallowSquash`', () => { + it('should return commits as valid', () => { + expect(validateCommitMessage('squash! feat(core): add feature')).toBe(VALID); + expect(validateCommitMessage('squash! fix: a bug')).toBe(VALID); + expect(validateCommitMessage('squash! fix a typo')).toBe(VALID); + }); }); describe('with `disallowSquash`', () => { @@ -191,21 +182,10 @@ 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.`; - - // Valid messages. + it('should return commits as valid', () => { expect(validateCommitMessage('fixup! feat(core): add feature')).toBe(VALID); expect(validateCommitMessage('fixup! fix: a bug')).toBe(VALID); - - // Invalid messages. - expect(validateCommitMessage('fixup! fix a typo')).toBe(INVALID); - expect(lastError).toContain('fixup! fix a typo'); - expect(lastError).toContain(errorMessage); - - expect(validateCommitMessage('fixup! fixup! fix: a bug')).toBe(INVALID); - expect(lastError).toContain('fixup! fixup! fix: a bug'); - expect(lastError).toContain(errorMessage); + expect(validateCommitMessage('fixup! fixup! fix: a bug')).toBe(VALID); }); }); diff --git a/dev-infra/commit-message/validate.ts b/dev-infra/commit-message/validate.ts index 453254dbf9..f8a62073f6 100644 --- a/dev-infra/commit-message/validate.ts +++ b/dev-infra/commit-message/validate.ts @@ -82,20 +82,29 @@ export function validateCommitMessage( //////////////////////////////////// // Checking revert, squash, fixup // //////////////////////////////////// + + // All revert commits are considered valid. if (commit.isRevert) { return true; } - if (commit.isSquash && options.disallowSquash) { - error('The commit must be manually squashed into the target commit'); - return false; + // All squashes are considered valid, as the commit will be squashed into another in + // the git history anyway, unless the options provided to not allow squash commits. + if (commit.isSquash) { + if (options.disallowSquash) { + error('The commit must be manually squashed into the target commit'); + return false; + } + return true; } - // 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 && options.nonFixupCommitHeaders) { - if (!options.nonFixupCommitHeaders.includes(commit.header)) { + // Fixups commits are considered valid, unless nonFixupCommitHeaders are provided to check + // against. If `nonFixupCommitHeaders` is not empty, we 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), otherwise we assume this verification will happen in another + // check. + if (commit.isFixup) { + if (options.nonFixupCommitHeaders && !options.nonFixupCommitHeaders.includes(commit.header)) { error( 'Unable to find match for fixup commit among prior commits: ' + (options.nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-')); @@ -132,12 +141,6 @@ export function validateCommitMessage( // Checking commit body // ////////////////////////// - // Commit bodies are not checked for fixups and squashes as they will be squashed into - // another commit in the history anyway. - if (commit.isFixup || commit.isSquash) { - return true; - } - if (commit.bodyWithoutLinking.trim().length < config.minBodyLength) { error(`The commit message body does not meet the minimum length of ${ config.minBodyLength} characters`);