From 1c63070ddf4756f3456e66e1bad3186eb368cf92 Mon Sep 17 00:00:00 2001 From: "Kenta Goto (k.goto)" <24818752+go-to-k@users.noreply.github.com> Date: Wed, 4 Sep 2024 05:55:07 +0900 Subject: [PATCH] fix(prlint): a review label doesn't appear when a PR is approved if there are too many comments (#31290) ### Issue # (if applicable) Closes #31294 . ### Reason for this change I've reviewed and approved [this PR](https://github.com/aws/aws-cdk/pull/30920) as a Trusted Community Reviewer. But it doesn't get the `pr/needs-maintainer-review` label. It seems to be in `CHANGES_REQUESTED` state and `communityApproved` is also false in the job `PR Linter / validate-pr`. (Please see [this comment in the PR](https://github.com/aws/aws-cdk/pull/30920#issuecomment-2324932936).) I checked [the prlint's log](https://github.com/aws/aws-cdk/blob/main/tools/@aws-cdk/prlint/lint.ts#L377) in [the GitHub Actions output](https://github.com/aws/aws-cdk/actions/runs/10669155243/job/29570426536), and it appears that there is too much history (such as comments) to get all the latest data. [List reviews for a pull request](https://docs.github.com/en/rest/pulls/reviews?apiVersion=2022-11-28#list-reviews-for-a-pull-request) in GitHub API can get 30 items per page, however, [prlint is not implemented to handle pagination](https://github.com/aws/aws-cdk/blob/main/tools/%40aws-cdk/prlint/lint.ts#L376). ```ts private async assessNeedsReview( pr: Pick, ): Promise { const reviews = await this.client.pulls.listReviews(this.prParams); ``` Therefore, when there are **more than 30 comments or change requests**, the review label is no longer displayed. ### Description of changes Use pagination for listReviews in the octokit library. https://github.com/octokit/octokit.js?tab=readme-ov-file#pagination before ```ts await this.client.pulls.listReviews(this.prParams); ``` after ```ts await this.client.paginate(this.client.pulls.listReviews, this.prParams); ``` ### Description of how you validated changes ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- tools/@aws-cdk/prlint/lint.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/@aws-cdk/prlint/lint.ts b/tools/@aws-cdk/prlint/lint.ts index 9097dd5516a4c..0cb1b89621a24 100644 --- a/tools/@aws-cdk/prlint/lint.ts +++ b/tools/@aws-cdk/prlint/lint.ts @@ -373,17 +373,17 @@ export class PullRequestLinter { private async assessNeedsReview( pr: Pick, ): Promise { - const reviews = await this.client.pulls.listReviews(this.prParams); - console.log(JSON.stringify(reviews.data)); + const reviewsData = await this.client.paginate(this.client.pulls.listReviews, this.prParams); + console.log(JSON.stringify(reviewsData)); // 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( + const maintainerRequestedChanges = reviewsData.some( review => review.author_association === 'MEMBER' && review.user?.login !== 'aws-cdk-automation' && review.state === 'CHANGES_REQUESTED', ); - const maintainerApproved = reviews.data.some( + const maintainerApproved = reviewsData.some( review => review.author_association === 'MEMBER' && review.state === 'APPROVED', ); @@ -403,7 +403,7 @@ export class PullRequestLinter { // be dismissed by a maintainer to respect another reviewer's requested changes. // 5. Checking if any reviewers' most recent review requested changes // -> If so, the PR is considered to still need changes to meet community review. - const reviewsByTrustedCommunityMembers = reviews.data + const reviewsByTrustedCommunityMembers = reviewsData .filter(review => this.getTrustedCommunityMembers().includes(review.user?.login ?? '')) .filter(review => review.state !== 'PENDING' && review.state !== 'COMMENTED') .reduce((grouping, review) => { @@ -420,12 +420,12 @@ export class PullRequestLinter { ...grouping, [review.user!.login]: newest, }; - }, {} as Record); + }, {} as Record); console.log('raw data: ', JSON.stringify(reviewsByTrustedCommunityMembers)); const communityApproved = Object.values(reviewsByTrustedCommunityMembers).some(({state}) => state === 'APPROVED'); const communityRequestedChanges = !communityApproved && Object.values(reviewsByTrustedCommunityMembers).some(({state}) => state === 'CHANGES_REQUESTED') - const prLinterFailed = reviews.data.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review; + const prLinterFailed = reviewsData.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review; const userRequestsExemption = pr.labels.some(label => (label.name === Exemption.REQUEST_EXEMPTION || label.name === Exemption.REQUEST_CLARIFICATION)); console.log('evaluation: ', JSON.stringify({ draft: pr.draft,