Skip to content

Commit

Permalink
chore: add labels to pr based off comment content and labels already …
Browse files Browse the repository at this point in the history
…added (aws#24095)

This will help us find the PRs that need attention even when they don't pass the PR Linter, the build, and/or already have changes requested.

The GitHub action adds a label for cli tests needed, for clarification requested, and for exemptions requested.

The PR linter output has been updated with a message to let contributors know to use these. I added in both the review comment and the regular comment so that it's not easy to miss.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
TheRealAmazonKendra authored Feb 10, 2023
1 parent e89b9ef commit a066467
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/pr-linter-exemption-labeler.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: pr-linter-exemption-labeler
on:
issue_comment:
types:
- created
- edited
- deleted

jobs:
pr_commented:
name: PR Comment
if: ${{ (github.event.issue.pull_request) && (github.event.issue.state == 'open') }}
runs-on: ubuntu-latest
steps:
- uses: TheRealAmazonKendra/pr-linter-exemption-labeler@main
with:
github-token: ${{ secrets.PROJEN_GITHUB_TOKEN }}
11 changes: 8 additions & 3 deletions tools/@aws-cdk/prlint/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,17 @@ export class PullRequestLinter {
* @param existingReview The review created by a previous run of the linter.
*/
private async createOrUpdatePRLinterReview(failureMessages: string[], existingReview?: Review): Promise<void> {
const body = `The pull request linter fails with the following errors:${this.formatErrors(failureMessages)}PRs must pass status checks before we can provide a meaningful review.`;
const body = `The pull request linter fails with the following errors:${this.formatErrors(failureMessages)}`
+ '<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n'
+ 'If you would like to request an exemption from the status checks or clarification on feedback,'
+ ' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.';
if (!existingReview) {
await this.client.pulls.createReview({
...this.prParams,
body: 'The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons.' +
' If you believe this pull request should receive an exemption, please comment and provide a justification.',
body: 'The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons.'
+ ' If you believe this pull request should receive an exemption, please comment and provide a justification.'
+ '\n\n\nA comment requesting an exemption should contain the text `Exemption Request`.'
+ ' Additionally, if clarification is needed add `Clarification Request` to a comment.',
event: 'REQUEST_CHANGES',
})
}
Expand Down
16 changes: 12 additions & 4 deletions tools/@aws-cdk/prlint/test/lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ describe('integration tests required on features', () => {
await expect(prLinter.validate()).rejects.toThrow(
'The pull request linter fails with the following errors:' +
'\n\n\t❌ Features must contain a change to an integration test file and the resulting snapshot.' +
'\n\nPRs must pass status checks before we can provide a meaningful review.'
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
'If you would like to request an exemption from the status checks or clarification on feedback,' +
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.'
);
});

Expand Down Expand Up @@ -305,7 +307,9 @@ describe('integration tests required on features', () => {
await expect(prLinter.validate()).rejects.toThrow(
'The pull request linter fails with the following errors:' +
'\n\n\t❌ Features must contain a change to an integration test file and the resulting snapshot.' +
'\n\nPRs must pass status checks before we can provide a meaningful review.'
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
'If you would like to request an exemption from the status checks or clarification on feedback,' +
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.'
);
});

Expand Down Expand Up @@ -338,7 +342,9 @@ describe('integration tests required on features', () => {
await expect(prLinter.validate()).rejects.toThrow(
'The pull request linter fails with the following errors:' +
'\n\n\t❌ Fixes must contain a change to an integration test file and the resulting snapshot.' +
'\n\nPRs must pass status checks before we can provide a meaningful review.'
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
'If you would like to request an exemption from the status checks or clarification on feedback,' +
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.'
);
});

Expand Down Expand Up @@ -371,7 +377,9 @@ describe('integration tests required on features', () => {
await expect(prLinter.validate()).rejects.toThrow(
'The pull request linter fails with the following errors:' +
'\n\n\t❌ Fixes must contain a change to an integration test file and the resulting snapshot.' +
'\n\nPRs must pass status checks before we can provide a meaningful review.'
'\n\n<b>PRs must pass status checks before we can provide a meaningful review.</b>\n\n' +
'If you would like to request an exemption from the status checks or clarification on feedback,' +
' please leave a comment on this PR containing `Exemption Request` and/or `Clarification Request`.'
);
});

Expand Down

0 comments on commit a066467

Please sign in to comment.