Skip to content

Commit

Permalink
chore(prlint): enforce lowercase pr title (aws#29002)
Browse files Browse the repository at this point in the history
Enforces that the title looks like `chore(prlint): enforce lowercase pr title` and not `chore(prlint): Enforce lowercase pr title`. 

Why does this matter? It doesn't really. But [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) tells us to be consistent, and their examples are all in lowercase, and most of our pr titles are already lowercase.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Feb 7, 2024
1 parent 5996856 commit 10c9458
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 18 deletions.
38 changes: 23 additions & 15 deletions tools/@aws-cdk/prlint/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,20 +542,6 @@ export class PullRequestLinter {
testRuleSet: [{ test: featureContainsIntegTest }, { test: fixContainsIntegTest }],
});

validationCollector.validateRuleSet({
testRuleSet: [{ test: validateBreakingChangeFormat }],
});

validationCollector.validateRuleSet({
testRuleSet: [{ test: validateTitlePrefix }],
});
validationCollector.validateRuleSet({
testRuleSet: [{ test: validateTitleScope }],
});
validationCollector.validateRuleSet({
testRuleSet: [{ test: validateBranch }],
})

validationCollector.validateRuleSet({
exemption: shouldExemptBreakingChange,
exemptionMessage: `Not validating breaking changes since the PR is labeled with '${Exemption.BREAKING_CHANGE}'`,
Expand All @@ -570,7 +556,17 @@ export class PullRequestLinter {
validationCollector.validateRuleSet({
exemption: (pr) => pr.user?.login === 'aws-cdk-automation',
testRuleSet: [{ test: noMetadataChanges }],
})
});

validationCollector.validateRuleSet({
testRuleSet: [
{ test: validateBreakingChangeFormat },
{ test: validateTitlePrefix },
{ test: validateTitleScope },
{ test: validateTitleLowercase },
{ test: validateBranch },
],
});

await this.deletePRLinterComment();
try {
Expand Down Expand Up @@ -735,6 +731,18 @@ function validateTitleScope(pr: GitHubPr): TestResult {
return result;
}

function validateTitleLowercase(pr: GitHubPr): TestResult {
const result = new TestResult();
const start = pr.title.indexOf(':');
const firstLetter = pr.title.charAt(start + 2);
console.log(firstLetter, firstLetter.toLocaleLowerCase());
result.assessFailure(
firstLetter !== firstLetter.toLocaleLowerCase(),
'The first word of the pull request title should not be capitalized. If the title starts with a CDK construct, it should be in backticks "``".',
);
return result;
}

/**
* Check that the PR is not opened from main branch of author's fork
*
Expand Down
34 changes: 31 additions & 3 deletions tools/@aws-cdk/prlint/test/lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,34 @@ describe('commit message format', () => {
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validatePullRequestTarget(SHA)).resolves;
});

test('invalid capitalized title', async () => {
const issue = {
number: 1,
title: 'fix(aws-cdk-lib): Some title',
body: '',
labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-integ-test' }],
user: {
login: 'author',
},
};
const prLinter = configureMock(issue, undefined);
await expect(prLinter.validatePullRequestTarget(SHA)).rejects.toThrow(/The first word of the pull request title should not be capitalized. If the title starts with a CDK construct, it should be in backticks "``"/);
});

test('valid capitalized title with backticks', async () => {
const issue = {
number: 1,
title: 'fix(aws-cdk-lib): `CfnConstruct`',
body: '',
labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-integ-test' }],
user: {
login: 'author',
},
};
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validatePullRequestTarget(SHA)).resolves;
});
});

describe('ban breaking changes in stable modules', () => {
Expand Down Expand Up @@ -1018,11 +1046,11 @@ describe('integration tests required on features', () => {

test('with aws-cdk-automation author', async () => {
const pr = {
title: 'chore: Update regions',
title: 'chore: update regions',
number: 1234,
labels: [],
user: {
login: 'aws-cdk-automation'
login: 'aws-cdk-automation',
},
};

Expand All @@ -1032,7 +1060,7 @@ describe('integration tests required on features', () => {

test('with another author', async () => {
const pr = {
title: 'chore: Update regions',
title: 'chore: update regions',
number: 1234,
labels: [],
user: {
Expand Down

0 comments on commit 10c9458

Please sign in to comment.