Skip to content
This repository has been archived by the owner on Feb 19, 2022. It is now read-only.

Add Binance #79

Closed
3 of 4 tasks
deepbrook opened this issue Sep 1, 2017 · 15 comments
Closed
3 of 4 tasks

Add Binance #79

deepbrook opened this issue Sep 1, 2017 · 15 comments

Comments

@deepbrook
Copy link
Collaborator

deepbrook commented Sep 1, 2017

Url:
https://www.binance.com/restapipub.html

Tasks:

  • Tests
  • REST Wrapper
  • Pair Formatter
  • Interface
@mukeez
Copy link

mukeez commented Sep 3, 2017

Hi, Interested in picking up this ticket. new to open source :)

@deepbrook
Copy link
Collaborator Author

deepbrook commented Sep 4, 2017

Hey @Mukz95 !
Great to hear you want to contribute! I'd suggest to first get acquainted with the code base of BitEx. Take a look at how we do things around here. If you feel like you have a grasp of what's going on (or if you don't, do not hesitate to ask!) and fork the repository.
Start committing your code (be sure to read bitex/CONTRIBUTING.md for requirements) and once you're done simply submit your pull request.

I'll check it out as soon as I can and leave feedback if it needs improvements / fixes, or simply merge it if everything's fine.

Also note that you do not have to submit a PR containing ALL the above points, but must always supply tests for your new code (so you may submit a new PairFormatter with tests, without having to supply REST wrappers or interface functions, etc).

@mukeez
Copy link

mukeez commented Sep 4, 2017

Yes just trying to understand the codebase, I was wondering
if https://github.com/Mukz95/bitex/blob/master/tests/formatters.py tests pass? I was looking into the code didn't see how they were supposed to pass, unless i have missed something or are they meant to fail for now and pass when implementation takes place?

@deepbrook
Copy link
Collaborator Author

deepbrook commented Sep 5, 2017

Actually, I'm not sure - I've added them at some point and somewhat abandoned the tests on master right after - but these changes are all meant for the 2.0 release, hence you should develop from dev (there everything should work - there's also more tests).

Sorry it's all a little confusing. Trying to clean it up for excellent people like you willing to contribute!

@deepbrook deepbrook added this to the 2.0 milestone Sep 5, 2017
@deepbrook deepbrook removed this from the 2.0 milestone Nov 22, 2017
@HughMacdonald
Copy link
Contributor

@Mukz95 are you planning on continuing work on this one? If not, I would be interested in picking up the rest of this

@HughMacdonald
Copy link
Contributor

HughMacdonald commented Dec 15, 2017

Actually... I'm afraid that I already started on this one. Branch here: https://github.com/HughMacdonald/bitex/tree/binance_support

I've got the REST part working - I need to work through the interface class now.

@nlsdfnbch would you like me to PR the REST part, or wait until I can do both the REST and the Interface?

@deepbrook
Copy link
Collaborator Author

@HughMacdonald , either way is fine!

@HughMacdonald
Copy link
Contributor

Done. Initial REST api implementation in #120

@HughMacdonald
Copy link
Contributor

How do you like to deal with additional required parameters to methods?

For example, both order_status and cancel_order on Binance are going to require the pair that the order is on. Am I okay to add this to the method definition, or do you want to ensure that all method definitions are identical between exchanges, and insist on the extra parameters being passed in through kwargs?

Thanks

@HughMacdonald
Copy link
Contributor

Actually, now all in #122. I took it in a direction that I felt okay with - happy to change it if you'd prefer.

Binance has the basic methods implemented, but I want to go through and put additional methods in place.

@deepbrook
Copy link
Collaborator Author

Merged! Thanks @HughMacdonald!

@deepbrook deepbrook reopened this Dec 16, 2017
@metaperl
Copy link

If this has been merged, should it be listed under supported exchanges?

@deepbrook
Copy link
Collaborator Author

Yes it should be - I usually update it before a release, but if you want, feel free to add it to the table

@karlandoh
Copy link

Has this been merged yet? We are waiting.

@deepbrook
Copy link
Collaborator Author

It has been merged already. It may not be on pypi yet - feel free to pip install via the dev branch to get the latest version for now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants