Skip to content

Commit 1c63070

Browse files
authored
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](#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](#30920 (comment)).) 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<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number'>, ): Promise<void> { 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*
1 parent f3bc16c commit 1c63070

File tree

1 file changed

+7
-7
lines changed

1 file changed

+7
-7
lines changed

tools/@aws-cdk/prlint/lint.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -373,17 +373,17 @@ export class PullRequestLinter {
373373
private async assessNeedsReview(
374374
pr: Pick<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number'>,
375375
): Promise<void> {
376-
const reviews = await this.client.pulls.listReviews(this.prParams);
377-
console.log(JSON.stringify(reviews.data));
376+
const reviewsData = await this.client.paginate(this.client.pulls.listReviews, this.prParams);
377+
console.log(JSON.stringify(reviewsData));
378378

379379
// NOTE: MEMBER = a member of the organization that owns the repository
380380
// COLLABORATOR = has been invited to collaborate on the repository
381-
const maintainerRequestedChanges = reviews.data.some(
381+
const maintainerRequestedChanges = reviewsData.some(
382382
review => review.author_association === 'MEMBER'
383383
&& review.user?.login !== 'aws-cdk-automation'
384384
&& review.state === 'CHANGES_REQUESTED',
385385
);
386-
const maintainerApproved = reviews.data.some(
386+
const maintainerApproved = reviewsData.some(
387387
review => review.author_association === 'MEMBER'
388388
&& review.state === 'APPROVED',
389389
);
@@ -403,7 +403,7 @@ export class PullRequestLinter {
403403
// be dismissed by a maintainer to respect another reviewer's requested changes.
404404
// 5. Checking if any reviewers' most recent review requested changes
405405
// -> If so, the PR is considered to still need changes to meet community review.
406-
const reviewsByTrustedCommunityMembers = reviews.data
406+
const reviewsByTrustedCommunityMembers = reviewsData
407407
.filter(review => this.getTrustedCommunityMembers().includes(review.user?.login ?? ''))
408408
.filter(review => review.state !== 'PENDING' && review.state !== 'COMMENTED')
409409
.reduce((grouping, review) => {
@@ -420,12 +420,12 @@ export class PullRequestLinter {
420420
...grouping,
421421
[review.user!.login]: newest,
422422
};
423-
}, {} as Record<string, typeof reviews.data[0]>);
423+
}, {} as Record<string, typeof reviewsData[0]>);
424424
console.log('raw data: ', JSON.stringify(reviewsByTrustedCommunityMembers));
425425
const communityApproved = Object.values(reviewsByTrustedCommunityMembers).some(({state}) => state === 'APPROVED');
426426
const communityRequestedChanges = !communityApproved && Object.values(reviewsByTrustedCommunityMembers).some(({state}) => state === 'CHANGES_REQUESTED')
427427

428-
const prLinterFailed = reviews.data.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review;
428+
const prLinterFailed = reviewsData.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review;
429429
const userRequestsExemption = pr.labels.some(label => (label.name === Exemption.REQUEST_EXEMPTION || label.name === Exemption.REQUEST_CLARIFICATION));
430430
console.log('evaluation: ', JSON.stringify({
431431
draft: pr.draft,

0 commit comments

Comments
 (0)