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

Development #301

Merged
merged 19 commits into from
Aug 19, 2018
Merged

Development #301

merged 19 commits into from
Aug 19, 2018

Conversation

brokencog
Copy link

get_product_trades() was incorrectly setting default value of limit argument to string '', rather than int 0.

acontry and others added 18 commits June 14, 2017 21:18
Drop the mixed keyword/dict interface in favor of keywords only.
Add docstrings based on Google format.
Was just looking over your last commit and saw this small mistake, great work! I like the new generator approach to pagination.
Also add docstring comments about paginated request paramters.
This move was made so get_product_trades can
return a generator like all other paginated
API endpoints.
…anup

AuthenticatedClient overhaul, docstrings, and testing
Fix heartbeat subscribe message not containing sequence
@alimcmaster1
Copy link
Contributor

Hi what are you attempting to do here/ what's the issue?

@brokencog
Copy link
Author

Sorry for the slow response.

The issue seemed fairly simple -- the function get_product_trades() has several arguements, one of which is "limit". This arg was being assigned a default value of '' ... that is: limit='' ... this is causing a problem in the function when limit is used as an integer. I was attempting to change the function definition to "limit=0" ... however I suck at git (and don't really care to unsuck) and as a result don't know what has been pushed or pulled for review.

@alimcmaster1
Copy link
Contributor

Ive fixed this in #311

@brokencog
Copy link
Author

brokencog commented Aug 9, 2018 via email

@danpaquin
Copy link
Owner

This is fantastic. I merged the changes. Thank you all for your contribution on this.

@danpaquin danpaquin merged commit 9ddad7f into danpaquin:master Aug 19, 2018
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.

6 participants