Closed mattpolicastro closed 8 years ago
Hi, thanks for the pull request, looks good. Did you change the naming of variables? I'm thinking that might not be the best idea since people are actively using the code, at least some fallback should be provided.
Nope, just removed the default of 1000 from the max
parameter from each of the calls, and added a previous
param to the base function call to roll over previous API results. It would be easy to add the max = 1000
param back in, but that feels like it would defeat the purpose of these changes.
@skardhamar any further thoughts on this?
Hi @mattpolicastro, I haven't looked into the update yet. But removing the default 1000 would hopefully not destroy any existing implementation.
I trust that the update is solid, and have merged the request :)
I ended up ditching the notion of batches entirely in favour of maintaining the
max
andstart
params in a more common-sense fashion. Of all the possible results that can be fetched from the API, these changes will get them—and will subset based on the starting index position and maximum number specified by the user. Otherwise, default to fetch all.Would love to hear comments/concerns, especially given that this deviates from the standard behaviour elsewhere in the package.