docs: Update pagination examples to show single clear approach#1243
docs: Update pagination examples to show single clear approach#1243developerkunal merged 2 commits intomasterfrom
Conversation
- Remove confusing async iteration pattern (for await...of) - Standardize on manual pagination with page.data and hasNextPage() - Update both README.md and v5_MIGRATION_GUIDE.md - Fix Page class exports to be runtime accessible - Addresses issue #1193 about poorly explained pagination
|
Claude finished @developerkunal's task —— View job Review Complete
Overall Assessment❌ This pull request is nearly ready but has one unrelated code change that should be addressed before merging. SummaryThe documentation updates are excellent and successfully address issue #1193 by providing clear, consistent pagination examples. However, there's an unrelated code change in What needs to be fixed:
What's working well:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1243 +/- ##
========================================
Coverage 78.19% 78.20%
========================================
Files 354 356 +2
Lines 17742 17746 +4
Branches 9413 9555 +142
========================================
+ Hits 13874 13878 +4
Misses 3868 3868
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
README.md
Outdated
| ## Pagination | ||
|
|
||
| Some list endpoints are paginated. The SDK provides an iterator so that you can simply loop over the items: | ||
| Some list endpoints are paginated. You can manually iterate page-by-page: |
There was a problem hiding this comment.
Is there a reason we do not document how to explicitly pass pagination data?
There was a problem hiding this comment.
I have added that example as well!
Changes
This PR updates the pagination documentation to provide a single, clear approach for paginating through results, addressing confusion reported in issue #1193.
Documentation Updates:
for await (const item of response)pattern that doesn't actually work (Page doesn't implement async iteration)page.data,hasNextPage(), andgetNextPage()Code Quality Improvements:
hasNextPage()checksExports update:
src/management/index.tsto re-export all items fromexports.js, potentially exposing additional SDK functionality.Why this is important:
Users were confused by contradictory pagination examples in the documentation. Issue #1193 highlighted that the docs showed an async iteration pattern that doesn't actually work with the
Pageclass. This PR removes the non-working pattern and shows only the officially recommended approach.References
Testing
This is a documentation-only change. No code functionality has been modified.
Verification:
Documentation examples are clear and consistent
Shows the officially recommended pagination approach
Removes patterns that don't work and were causing confusion
Both README.md and v5_MIGRATION_GUIDE.md use the same pagination pattern
This change adds unit test coverage - N/A (documentation only)
This change adds integration test coverage - N/A (documentation only)
Checklist