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

Added pagination to EconItems#GetSchemaItemsForTF2Async() #105

Merged
merged 3 commits into from
Jul 10, 2020
Merged

Added pagination to EconItems#GetSchemaItemsForTF2Async() #105

merged 3 commits into from
Jul 10, 2020

Conversation

QuantumToasted
Copy link
Contributor

This fixes issue #104. I thought I'd just get the PR out of the way to save time. Verified with a modified unit test for the method which validates and uses the previously unused Next property in the response from the API.

The changing of Next to a uint? is a small breaking change, so if it would be preferred to keep it the same type and make other checks internally to know if we're "done" paginating, that could be done too.

If this is too breaking a change, a consideration for `uint` with a default value of 0 would work, but 0 is still a valid value for "start" on the request.
added a `uint? start = null` optional property.
This verifies that both a) the Next property is valid on each request, and b) that passing `start` along works as intended. This replaces the original test, but could be put alongside it instead.
@babelshift babelshift merged commit 8544805 into babelshift:master Jul 10, 2020
@babelshift
Copy link
Owner

Thanks for your contribution. Super helpful!

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