chore(prlint): core PRs are always pr/needs-maintainer-review (#26487)

Built the PR off of #26486 to hopefully avoid merge conflicts if we merge the other first ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Jul 24, 2023 · 7f8eeb3 · 7f8eeb3
1 parent 6ec9829
commit 7f8eeb3
Show file tree

Hide file tree

Showing 8 changed files with 180 additions and 160 deletions.
diff --git a/tools/@aws-cdk/prlint/.eslintrc.js b/tools/@aws-cdk/prlint/.eslintrc.js
@@ -0,0 +1,9 @@
+const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
+baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
+module.exports = {
+  ...baseConfig,
+  rules: {
+    'no-console': 'off',
+    'jest/valid-expect': 'off',
+  },
+};
diff --git a/tools/@aws-cdk/prlint/.gitignore b/tools/@aws-cdk/prlint/.gitignore
@@ -2,3 +2,4 @@
 *.js
 
 !decs.d.ts
+!.eslintrc.js
diff --git a/tools/@aws-cdk/prlint/index.ts b/tools/@aws-cdk/prlint/index.ts
@@ -1,8 +1,8 @@
 import * as core from '@actions/core';
 import * as github from '@actions/github';
 import { Octokit } from '@octokit/rest';
-import * as linter from './lint';
 import { StatusEvent, PullRequestEvent } from '@octokit/webhooks-definitions/schema';
+import * as linter from './lint';
 
 async function run() {
   const token: string = process.env.GITHUB_TOKEN!;
@@ -39,4 +39,4 @@ async function run() {
   }
 }
 
-run()
+void run();
diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts
@@ -1,13 +1,13 @@
+import { execSync } from 'child_process';
 import * as path from 'path';
 import { Octokit } from '@octokit/rest';
+import { Endpoints } from '@octokit/types';
 import { StatusEvent } from '@octokit/webhooks-definitions/schema';
-import { breakingModules } from './parser';
 import { findModulePath, moduleStability } from './module';
-import { Endpoints } from "@octokit/types";
-import { execSync } from 'child_process';
+import { breakingModules } from './parser';
 
 export type GitHubPr =
-  Endpoints["GET /repos/{owner}/{repo}/pulls/{pull_number}"]["response"]["data"];
+  Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response']['data'];
 
 export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)';
 
@@ -237,8 +237,8 @@ export class PullRequestLinter {
       await this.client.pulls.dismissReview({
         ...this.prParams,
         review_id: existingReview.id,
-        message: '✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.'
-      })
+        message: '✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.',
+      });
     }
   }
 
@@ -258,15 +258,15 @@ export class PullRequestLinter {
         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.',
+          + ' Additionally, if clarification is needed add `Clarification Request` to a comment.',
         event: 'REQUEST_CHANGES',
-      })
+      });
     }
 
     await this.client.issues.createComment({
       ...this.issueParams,
       body,
-    })
+    });
 
     throw new LinterError(body);
   }
@@ -296,7 +296,7 @@ export class PullRequestLinter {
   private async communicateResult(result: ValidationCollector): Promise<void> {
     const existingReview = await this.findExistingReview();
     if (result.isValid()) {
-      console.log("✅  Success");
+      console.log('✅  Success');
       await this.dismissPRLinterReview(existingReview);
     } else {
       await this.createOrUpdatePRLinterReview(result.errors, existingReview);
@@ -342,27 +342,27 @@ export class PullRequestLinter {
    *   7. It links to a p2 issue and has an approved community review
    */
   private async assessNeedsReview(
-    pr: Pick<GitHubPr, "mergeable_state" | "draft" | "labels" | "number">,
+    pr: Pick<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number'>,
   ): Promise<void> {
     const reviews = await this.client.pulls.listReviews(this.prParams);
     // NOTE: MEMBER = a member of the organization that owns the repository
     // COLLABORATOR = has been invited to collaborate on the repository
     const maintainerRequestedChanges = reviews.data.some(
       review => review.author_association === 'MEMBER'
         && review.user?.login !== 'aws-cdk-automation'
-        && review.state === 'CHANGES_REQUESTED'
+        && review.state === 'CHANGES_REQUESTED',
     );
     const maintainerApproved = reviews.data.some(
       review => review.author_association === 'MEMBER'
-        && review.state === 'APPROVED'
+        && review.state === 'APPROVED',
     );
     const communityRequestedChanges = reviews.data.some(
       review => this.getTrustedCommunityMembers().includes(review.user?.login ?? '')
-        && review.state !== 'APPROVED' // community members cannot request changes
-    )
+        && review.state !== 'APPROVED', // community members cannot request changes
+    );
     const communityApproved = reviews.data.some(
       review => this.getTrustedCommunityMembers().includes(review.user?.login ?? '')
-        && review.state === 'APPROVED'
+        && review.state === 'APPROVED',
     );
 
     const prLinterFailed = reviews.data.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review;
@@ -394,7 +394,11 @@ export class PullRequestLinter {
       readyForReview = false;
     }
 
-    if (readyForReview && (fixesP1 || communityApproved)) {
+    // needs-maintainer-review means one of the following
+    // 1) fixes a p1 bug
+    // 2) is already community approved
+    // 3) is authored by a core team member
+    if (readyForReview && (fixesP1 || communityApproved || pr.labels.some(label => label.name === 'contribution/core'))) {
       this.addLabel('pr/needs-maintainer-review', pr);
       this.removeLabel('pr/needs-community-review', pr);
     } else if (readyForReview && !fixesP1) {
@@ -406,7 +410,7 @@ export class PullRequestLinter {
     }
   }
 
-  private addLabel(label: string, pr: Pick<GitHubPr, "labels" | "number">) {
+  private addLabel(label: string, pr: Pick<GitHubPr, 'labels' | 'number'>) {
     // already has label, so no-op
     if (pr.labels.some(l => l.name === label)) { return; }
     console.log(`adding ${label} to pr ${pr.number}`);
@@ -420,7 +424,7 @@ export class PullRequestLinter {
     });
   }
 
-  private removeLabel(label: string, pr: Pick<GitHubPr, "labels" | "number">) {
+  private removeLabel(label: string, pr: Pick<GitHubPr, 'labels' | 'number'>) {
     // does not have label, so no-op
     if (!pr.labels.some(l => l.name === label)) { return; }
     console.log(`removing ${label} to pr ${pr.number}`);
@@ -439,7 +443,7 @@ export class PullRequestLinter {
   private getTrustedCommunityMembers(): string[] {
     if (this.trustedCommunity.length > 0) { return this.trustedCommunity; }
 
-    const wiki = execSync('curl https://raw.githubusercontent.com/wiki/aws/aws-cdk/Introducing-CDK-Community-PR-Reviews.md', { encoding: 'utf-8'}).toString();
+    const wiki = execSync('curl https://raw.githubusercontent.com/wiki/aws/aws-cdk/Introducing-CDK-Community-PR-Reviews.md', { encoding: 'utf-8' }).toString();
     const rawMdTable = wiki.split('<!--section-->')[1].split('\n').filter(l => l !== '');
     for (let i = 2; i < rawMdTable.length; i++) {
       this.trustedCommunity.push(rawMdTable[i].split('|')[1].trim());
@@ -460,48 +464,48 @@ export class PullRequestLinter {
     console.log(`⌛  Fetching files for PR number ${number}`);
     const files = await this.client.paginate(this.client.pulls.listFiles, this.prParams);
 
-    console.log("⌛  Validating...");
+    console.log('⌛  Validating...');
 
     const validationCollector = new ValidationCollector(pr, files);
 
     validationCollector.validateRuleSet({
       exemption: shouldExemptReadme,
       exemptionMessage: `Not validating README changes since the PR is labeled with '${Exemption.README}'`,
-      testRuleSet: [ { test: featureContainsReadme } ],
+      testRuleSet: [{ test: featureContainsReadme }],
     });
 
     validationCollector.validateRuleSet({
       exemption: shouldExemptTest,
       exemptionMessage: `Not validating test changes since the PR is labeled with '${Exemption.TEST}'`,
-      testRuleSet: [ { test: featureContainsTest }, { test: fixContainsTest } ],
+      testRuleSet: [{ test: featureContainsTest }, { test: fixContainsTest }],
     });
 
     validationCollector.validateRuleSet({
       exemption: shouldExemptIntegTest,
       exemptionMessage: `Not validating integration test changes since the PR is labeled with '${Exemption.INTEG_TEST}'`,
-      testRuleSet: [ { test: featureContainsIntegTest}, { test: fixContainsIntegTest } ]
+      testRuleSet: [{ test: featureContainsIntegTest }, { test: fixContainsIntegTest }],
     });
 
     validationCollector.validateRuleSet({
-      testRuleSet: [ { test: validateBreakingChangeFormat } ]
+      testRuleSet: [{ test: validateBreakingChangeFormat }],
     });
 
     validationCollector.validateRuleSet({
-      testRuleSet: [ { test: validateTitlePrefix } ]
+      testRuleSet: [{ test: validateTitlePrefix }],
     });
     validationCollector.validateRuleSet({
-      testRuleSet: [ { test: validateTitleScope } ]
-    })
+      testRuleSet: [{ test: validateTitleScope }],
+    });
 
     validationCollector.validateRuleSet({
       exemption: shouldExemptBreakingChange,
       exemptionMessage: `Not validating breaking changes since the PR is labeled with '${Exemption.BREAKING_CHANGE}'`,
-      testRuleSet: [ { test: assertStability } ]
+      testRuleSet: [{ test: assertStability }],
     });
 
     validationCollector.validateRuleSet({
       exemption: shouldExemptCliIntegTested,
-      testRuleSet: [ { test: noCliChanges } ],
+      testRuleSet: [{ test: noCliChanges }],
     });
 
     await this.deletePRLinterComment();
@@ -527,31 +531,31 @@ export class PullRequestLinter {
 }
 
 function isPkgCfnspec(pr: GitHubPr): boolean {
-  return pr.title.indexOf("(cfnspec)") > -1;
+  return pr.title.indexOf('(cfnspec)') > -1;
 }
 
 function isFeature(pr: GitHubPr): boolean {
-  return pr.title.startsWith("feat")
+  return pr.title.startsWith('feat');
 }
 
 function isFix(pr: GitHubPr): boolean {
-  return pr.title.startsWith("fix")
+  return pr.title.startsWith('fix');
 }
 
 function testChanged(files: GitHubFile[]): boolean {
-  return files.filter(f => f.filename.toLowerCase().includes("test")).length != 0;
+  return files.filter(f => f.filename.toLowerCase().includes('test')).length != 0;
 }
 
 function integTestChanged(files: GitHubFile[]): boolean {
   return files.filter(f => f.filename.toLowerCase().match(/integ.*.ts$/)).length != 0;
 }
 
 function integTestSnapshotChanged(files: GitHubFile[]): boolean {
-  return files.filter(f => f.filename.toLowerCase().includes(".snapshot")).length != 0;
+  return files.filter(f => f.filename.toLowerCase().includes('.snapshot')).length != 0;
 }
 
 function readmeChanged(files: GitHubFile[]): boolean {
-  return files.filter(f => path.basename(f.filename) == "README.md").length != 0;
+  return files.filter(f => path.basename(f.filename) == 'README.md').length != 0;
 }
 
 function featureContainsReadme(pr: GitHubPr, files: GitHubFile[]): TestResult {
@@ -627,7 +631,7 @@ function validateBreakingChangeFormat(pr: GitHubPr, _files: GitHubFile[]): TestR
   const m = re.exec(body ?? '');
   if (m) {
     result.assessFailure(!m[0].startsWith('BREAKING CHANGE: '), `Breaking changes should be indicated by starting a line with 'BREAKING CHANGE: ', variations are not allowed. (found: '${m[0]}').`);
-    result.assessFailure(m[0].slice('BREAKING CHANGE:'.length).trim().length === 0, `The description of the first breaking change should immediately follow the 'BREAKING CHANGE: ' clause.`);
+    result.assessFailure(m[0].slice('BREAKING CHANGE:'.length).trim().length === 0, 'The description of the first breaking change should immediately follow the \'BREAKING CHANGE: \' clause.');
     const titleRe = /^[a-z]+\([0-9a-z-_]+\)/;
     result.assessFailure(!titleRe.exec(title), 'The title of this pull request must specify the module name that the first breaking change should be associated to.');
   }
@@ -643,7 +647,7 @@ function validateTitlePrefix(pr: GitHubPr): TestResult {
   const m = titleRe.exec(pr.title);
   result.assessFailure(
     !m,
-    "The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.");
+    'The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.');
   return result;
 }
 
@@ -654,7 +658,7 @@ function validateTitlePrefix(pr: GitHubPr): TestResult {
  */
 function validateTitleScope(pr: GitHubPr): TestResult {
   const result = new TestResult();
-  const scopesExemptFromThisRule = [ 'aws-cdk-lib' ];
+  const scopesExemptFromThisRule = ['aws-cdk-lib'];
   // 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>)"

diff --git a/tools/@aws-cdk/prlint/module.ts b/tools/@aws-cdk/prlint/module.ts
@@ -1,6 +1,6 @@
+import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as glob from 'glob';
-import * as path from 'path';
 
 let awsCdkLibPath: string;
 const modules: string[] = [];
@@ -9,7 +9,7 @@ const awsCdkLibModules: string[] = [];
 export function findModulePath(fuzz: string): string {
   discoverModules();
 
-  const regex = new RegExp(`[-_/]${fuzz}($|-alpha)`)
+  const regex = new RegExp(`[-_/]${fuzz}($|-alpha)`);
   const matched = [
     ...modules.filter(m => regex.test(m)),
     ...(awsCdkLibModules.some(m => regex.test(m)) ? [awsCdkLibPath] : []),
@@ -32,7 +32,7 @@ function discoverModules() {
       throw new Error('env REPO_ROOT must be set');
     }
     const repoRoot = process.env.REPO_ROOT;
-    const lernaConfig = require(path.join(repoRoot, 'lerna.json'));
+    const lernaConfig = require(path.join(repoRoot, 'lerna.json')); // eslint-disable-line @typescript-eslint/no-require-imports
     const searchPaths: string[] = lernaConfig.packages;
     awsCdkLibPath = path.join(repoRoot, 'packages', 'aws-cdk-lib');
     searchPaths.forEach(p => {
@@ -67,6 +67,6 @@ export function moduleStability(loc: string): 'stable' | 'experimental' | undefi
   if (!fs.existsSync(path.join(loc, 'package.json'))) {
     throw new Error(`unexpected: no package.json found at location "${loc}"`);
   }
-  const pkgjson = require(path.join(loc, 'package.json'));
+  const pkgjson = require(path.join(loc, 'package.json')); // eslint-disable-line @typescript-eslint/no-require-imports
   return pkgjson.stability;
 }
diff --git a/tools/@aws-cdk/prlint/package.json b/tools/@aws-cdk/prlint/package.json
@@ -25,7 +25,12 @@
     "@types/jest": "^29.5.3",
     "jest": "^29.6.1",
     "make-runnable": "^1.4.1",
-    "typescript": "~5.1.6"
+    "typescript": "~5.1.6",
+    "eslint": "^7.32.0",
+    "eslint-import-resolver-node": "^0.3.7",
+    "eslint-import-resolver-typescript": "^2.7.1",
+    "eslint-plugin-import": "^2.27.5",
+    "eslint-plugin-jest": "^24.7.0"
   },
   "jest": {
     "testMatch": [
@@ -37,6 +42,7 @@
     "test": "jest",
     "build+test": "npm run build && npm run test",
     "build+test+extract": "npm run build+test",
-    "build+extract": "npm run build"
+    "build+extract": "npm run build",
+    "lint": "tsc -b && eslint . --ext=.ts"
   }
 }