Skip to content

🐛 fix: fetch all pages data to avoid bypass check fail#30

Merged
SigureMo merged 12 commits intomainfrom
fetch-all-pages-data-to-avoid-mismatch
Jun 23, 2025
Merged

🐛 fix: fetch all pages data to avoid bypass check fail#30
SigureMo merged 12 commits intomainfrom
fetch-all-pages-data-to-avoid-mismatch

Conversation

@SigureMo
Copy link
Contributor

每个 REST API 都获取全部 pages 以免部分 event 缺失导致匹配失败

@SigureMo SigureMo requested a review from Copilot June 23, 2025 16:27
@SigureMo SigureMo added the ci-bypass: example Skip CI check for example workflow label Jun 23, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that all paginated API endpoints retrieve every page of data, preventing incomplete event or label checks.

  • Introduce a reusable withAllPages helper in utils.ts for full pagination.
  • Replace single-page octokit calls with withAllPages in label.ts, comment.ts, and approve.ts.
  • Adjust response handling to work directly with arrays instead of .data wrappers.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
src/rules/utils.ts Added withAllPages helper and Response interface to abstract pagination logic.
src/rules/label.ts Swapped listEvents and listLabelsOnIssue for withAllPages and removed .data usage.
src/rules/comment.ts Replaced listComments with withAllPages, mapping directly over the returned array.
src/rules/approve.ts Updated listReviews to use withAllPages and flattened the result into an array.
Comments suppressed due to low confidence (5)

src/rules/utils.ts:86

  • Add JSDoc above withAllPages to explain the purpose, parameters, and return value of the helper.
export function withAllPages<T, U>(

src/rules/utils.ts:86

  • Add unit tests to verify withAllPages correctly accumulates items across multiple pages, including edge cases.
export function withAllPages<T, U>(

src/rules/label.ts:33

  • [nitpick] Rename allEventsResponse to something like allEvents to reflect that this variable is now an array, not an API response object.
    const allEventsResponse = await withAllPages(

src/rules/label.ts:41

  • [nitpick] Rename allLabelsResponse to allLabels to avoid confusion since it contains a flat array of label objects.
    const allLabelsResponse = await withAllPages(

src/rules/approve.ts:27

  • [nitpick] Rename allReviewResponse to allReviews or reviews to reflect that it holds an array of review records.
    const allReviewResponse = await withAllPages(

Comment on lines 42 to 46
const allCommentResponseOld = await octokit.rest.issues.listComments({
owner,
repo,
issue_number: number,
})
Copy link

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the unused allCommentResponseOld call to avoid redundant API requests and dead code.

Suggested change
const allCommentResponseOld = await octokit.rest.issues.listComments({
owner,
repo,
issue_number: number,
})
// Removed unused allCommentResponseOld variable and its associated API call.

Copilot uses AI. Check for mistakes.
@SigureMo SigureMo removed the ci-bypass: example Skip CI check for example workflow label Jun 23, 2025
@SigureMo
Copy link
Contributor Author

/bypass all

@SigureMo SigureMo merged commit 65a5a5c into main Jun 23, 2025
10 checks passed
@SigureMo SigureMo deleted the fetch-all-pages-data-to-avoid-mismatch branch June 23, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants