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

v0.2.3 #50

Merged
merged 8 commits into from
Mar 1, 2017
Merged

v0.2.3 #50

merged 8 commits into from
Mar 1, 2017

Conversation

s4w3d0ff
Copy link
Owner

the docs does not mention "fill or kill" for the move order, so i left it out

@s4w3d0ff s4w3d0ff added the enhancement New feature or bug fix label Feb 10, 2017
Copy link
Contributor

@kdb424 kdb424 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought that I had seen that before. It may have been my mistake, or changed. I agree with this change.

@s4w3d0ff
Copy link
Owner Author

I was also thinking of changing the argument names to match what polo has, example: 'fill_or_kill' should probably be 'fillOrKill' even tho it is not pep8... I personally have an easier time reading between the api docs and code if it is as close to the actual docs as possible.

@s4w3d0ff s4w3d0ff closed this Feb 10, 2017
@s4w3d0ff s4w3d0ff reopened this Feb 10, 2017
@kdb424
Copy link
Contributor

kdb424 commented Feb 10, 2017

Considering the rest of your library doesn't follow full PEP8, I can see that as an acceptable change.

@s4w3d0ff s4w3d0ff changed the title moveOrder adv options moveOrder adv options and withdraw paymentId Feb 10, 2017
@s4w3d0ff s4w3d0ff changed the title moveOrder adv options and withdraw paymentId v0.2.3 Feb 12, 2017
@s4w3d0ff
Copy link
Owner Author

s4w3d0ff commented Feb 13, 2017

We should have an 'orderType' argument for the advanced buy/sell/move orders instead of an argument for each type. It would reduce code and allow easier updates in the future if polo decides to add more. I'II make a commit to this branch with the changes tonight.

makes it easy to add/remove new/old order types if poloniex decides to make more changes
a little bit better logging access, but not much...
@s4w3d0ff s4w3d0ff merged commit 11f6feb into master Mar 1, 2017
@s4w3d0ff s4w3d0ff deleted the adv-orders branch March 1, 2017 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants