Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(prlint): require consistent scope (without aws-) #23672

Merged
merged 2 commits into from Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions tools/@aws-cdk/prlint/lint.ts
Expand Up @@ -306,6 +306,9 @@ export class PullRequestLinter {
validationCollector.validateRuleSet({
testRuleSet: [ { test: validateTitlePrefix } ]
});
validationCollector.validateRuleSet({
testRuleSet: [ { test: validateTitleScope } ]
})

validationCollector.validateRuleSet({
exemption: shouldExemptBreakingChange,
Expand Down Expand Up @@ -446,6 +449,29 @@ function hasLabel(pr: GitHubPr, labelName: string): boolean {
return result;
};

/**
* Check that the PR title uses the typical convention for package names.
*
* For example, "fix(s3)" is preferred over "fix(aws-s3)".
*/
function validateTitleScope(pr: GitHubPr): TestResult {
const result = new TestResult();
// Specific commit types are handled by `validateTitlePrefix`. This just checks whether
// the scope includes an `aws-` prefix or not.
// Group 1: Scope with parens - "(aws-<name>)"
// Group 2: Scope name - "aws-<name>"
// Group 3: Preferred scope name - "<name>"
const titleRe = /^\w+(\((aws-([\w_-]+))\))?: /;
const m = titleRe.exec(pr.title);
if (m) {
result.assessFailure(
!!(m[2] && m[3]),
`The title of the pull request should omit 'aws-' from the name of modified packages. Use '${m[3]}' instead of '${m[2]}'.`,
);
}
return result;
}

function assertStability(pr: GitHubPr, _files: GitHubFile[]): TestResult {
const title = pr.title;
const body = pr.body;
Expand Down
57 changes: 57 additions & 0 deletions tools/@aws-cdk/prlint/test/lint.test.ts
Expand Up @@ -58,6 +58,63 @@ describe('breaking changes format', () => {
});
});

describe('commit message format', () => {
test('valid scope', async () => {
const issue = {
number: 1,
title: 'chore(s3): some title',
body: '',
labels: [],
};
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validate()).resolves;
});

test('invalid scope with aws- prefix', async () => {
const issue = {
number: 1,
title: 'fix(aws-s3): some title',
body: '',
labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-integ-test' }],
};
const prLinter = configureMock(issue, undefined);
await expect(prLinter.validate()).rejects.toThrow(/The title of the pull request should omit 'aws-' from the name of modified packages. Use 's3' instead of 'aws-s3'./);
});

test('valid scope with aws- in summary and body', async () => {
const issue = {
number: 1,
title: 'docs(s3): something aws-s3',
body: 'something aws-s3',
labels: [],
};
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validate()).resolves;
});

test('valid with missing scope', async () => {
const issue = {
number: 1,
title: 'docs: something aws-s3',
body: '',
labels: [],
};
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validate()).resolves;
});

test.each(['core', 'prlint', 'awslint'])('valid scope for packages that dont use aws- prefix', async (scope) => {
const issue = {
number: 1,
title: `chore(${scope}): some title`,
body: '',
labels: []
};
const prLinter = configureMock(issue, undefined);
expect(await prLinter.validate()).resolves;
})
});

describe('ban breaking changes in stable modules', () => {
test('breaking change in stable module', async () => {
const issue = {
Expand Down