Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor paginatorToArray to use AsyncIterable instead of AsyncIterableIterator #745

Merged
merged 1 commit into from
Oct 1, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,8 @@
return true
}

async function paginatorToArray<T, R>(

Check failure on line 414 in packages/core/src/github.ts

View workflow job for this annotation

GitHub Actions / build

The change from `AsyncIterableIterator<T>` to `AsyncIterable<T>` may cause issues if the `iterator` object was being used with `next()` method, which is not available on `AsyncIterable<T>`. Please ensure that the `iterator` object is only being used for iteration and not for manual control of the iteration process. 🧐

Check failure on line 414 in packages/core/src/github.ts

View workflow job for this annotation

GitHub Actions / build

The change in type may affect the functionality of the `paginatorToArray` function. If the function was expecting an `AsyncIterableIterator<T>`, changing it to `AsyncIterable<T>` might break the function if it was using properties or methods specific to `AsyncIterableIterator<T>`. Please ensure that the function can handle `AsyncIterable<T>` without any issues. 😊

Check failure on line 414 in packages/core/src/github.ts

View workflow job for this annotation

GitHub Actions / build

The change in the type of the `iterator` parameter in the `paginatorToArray` function has been made without any explanation or review comments. It would be helpful to provide a rationale for this change to help reviewers understand the intent behind it. This can also help in catching potential issues that might arise due to this change. 😇
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
iterator: AsyncIterableIterator<T>,
iterator: AsyncIterable<T>,
count: number,
iteratorItem: (item: T) => R[],
elementFilter?: (item: R) => boolean
Expand Down