8000 New Exchange: Coinzip by iammac360 · Pull Request #4797 · ccxt/ccxt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

New Exchange: Coinzip #4797

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 47 commits into from
Closed

New Exchange: Coinzip #4797

wants to merge 47 commits into from

Conversation

iammac360
Copy link

Hi, we recently launch Coinzip(https://www.coinzip.co/) and we want to integrate with ccxt. Hope you can accept our PR. Thanks

@kroitor
Copy link
Member
kroitor commented Mar 7, 2019 •
edited
Loading

Hi!

First of, thanks for the PR! Appreciate your involvement!

There's a few issues with it, however, namely, on the language portability... Have you read this:

... and, especially, this:

? )

We need to make sure that it passes the tests and syntactic checks. After that we will be able to verify that all methods are implemented properly, but tests first.

@kroitor kroitor self-assigned this Mar 7, 2019
@kroitor
Copy link
Member
kroitor commented Mar 7, 2019

You might also want to read this to run the same set of tests locally: https://github.com/ccxt/ccxt/blob/master/CONTRIBUTING.md#how-to-build--run-tests-on-your-local-machine

@iammac360
Copy link
Author

Hi!

First of, thanks for the PR! Appreciate your involvement!

There's a few issues with it, however, namely, on the language portability... Have you read this:

... and, especially, this:

? )

We need to make sure that it passes the tests and syntactic checks. After that we will be able to verify that all methods are implemented properly, but tests first.

Thanks for the feedback. I guess I have to refactor all the const declation to let. I mislook the coding standards stated here https://github.com/ccxt/ccxt/blob/master/CONTRIBUTING.md#derived-exchange-classes and coded on my own way which is derived from a functional, immutable style which is why I always use const.
I also forgot to run the linters and only run the tests.

Though I've run this script on my local machine:

node run-tests.js --js --php --python coinzip

Everythings green on my last run, but I guess I need to remove the --language option also or did I just missed something?? I'll just rerun this again once I'm home.

I'll update this PR once I've complied to all the standards and everythings green.

@kroitor
Copy link
Member
kroitor commented Mar 7, 2019

Thanks for the feedback. I guess I have to refactor all the const declation to let.

This isn't necessary, we actually do support const already ) So, yeah, we prefer const as well. I will edit that particular rule in the guidelines. But the rest of them still apply.

kroitor added a commit that referenced this pull request Mar 7, 2019
@kroitor
added a clarification on const vs let vs var #4797
e3044da
@kroitor
Copy link
Member
kroitor commented Mar 7, 2019

Ok, I've updated the guidelines – const and let are ok (if used properly), but not var ;)

@iammac360
Copy link
Author

Thanks for the feedback. I guess I have to refactor all the const declation to let.

This isn't necessary, we actually do support const already ) So, yeah, we prefer const as well. I will edit that particular rule in the guidelines. But the rest of them still apply.

Okay good to hear that. Thanks again. I'll review the CI errors also to see where that Issue is.

@iammac360
Copy link
Author

Hi @kroitor, I've updated my fork and fixed the failing test. Can you please verify it. Thank you.

@kroitor
Copy link
Member
kroitor commented Mar 11, 2019

Hi Mark! Thank you for the effort and for your time on this! I will review the work that you've done asap and will post my comments on it. In the meantime, if you can help clean up this PR a bit more – that would really speed up things, namely, we need to restore the generated files to their original versions – no need to commit those, as most of them will be overwritten upon building on travis anyway.

From this list:

.editorconfig   // ←-------- we'd take this file
README.md
ccxt.d.ts
ccxt.js
dist/ccxt.browser.js
js/coinzip.js  // ←------------- and we need this file
php/Exchange.php
php/coinzip.php
python/ccxt/__init__.py
python/ccxt/async_support/__init__.py
python/ccxt/async_support/coinzip.py
python/ccxt/coinzip.py
wiki/Exchange-Markets-By-Country.md
wiki/Exchange-Markets.md
wiki/Manual.md

All the other files are generated, so you can revert them to the master version with this command in your local repo:

git checkout master -- README.md ccxt.d.ts ccxt.js dist/ccxt.browser.js php/Exchange.php php/coinzip.php python/ccxt/__init__.py python/ccxt/async_support/__init__.py python/ccxt/async_support/coinzip.py python/ccxt/coinzip.py wiki/Exchange-Markets-By-Country.md wiki/Exchange-Markets.md wiki/Manual.md
git commit -am 'reverted generated files to master'
git push

Thx again! Will get back to you on this soon!

@iammac360
Copy link
Author

HI @kroitor, I've already updated my PR. Can you please check it again. Thanks!

iammac360 added 16 commits March 13, 2019 16:50
@iammac360
Initial Implementation of coinzip wrapper
afb3eb5
@iammac360
Add .editorconfig to automatically set and comply with the spacing rules
2f2b8d7
@iammac360
Initial implementation of coinzip wrapper
f5dc6ad
@iammac360
Update node modules
d216027
@iammac360
Implement fetchMarkets() method for coinzip
91b7dbe
@iammac360
Implement fetchTickers() method for coinzip
a83e0f2
@iammac360
Implement fetchTicker() method for coinzip
503fcc5
@iammac360
Implement fetchOrderBook() method for coinzip
2ec6ef8
@iammac360
Implement fetchOHLCV() method for coinzip
e2e0775
@iammac360
Implement fetchTrades() method for coinzip
0a24e49
@iammac360
Fix failing tests
8c384ce
@iammac360
Implement fetchBalance() method for coinzip. Also refactor sign() method
b5cb790
@iammac360
Implement createOrder() method for coinzip.
bc4694c
@iammac360
Implement cancelOrder() method for coinzip.
9263ef1
@iammac360
Implement fetchOrder() method for coinzip.
5a05506
@iammac360
Implement fetchOrders(), fetchOpenOrders() and fetchClosedOrder() met…
8457fe0 
…hods for coinzip.
iammac360 added 24 commits March 13, 2019 16:50
@iammac360
Implement fetch_order() method for coinzip python client
f9ed6b5
@iammac360
Implement fetch_orders() method for coinzip python client
956c8e5
@iammac360
Implement fetch_open_orders(), fetch_closed_orders() and fetch_my_tra…
c607912 
…des() methods for coinzip python client
@iammac360
Adds editor config for php
8500a05
@iammac360
Initial coinzip php implementation
ed0bcd3
@iammac360
Implement fetch_markets() method for coinzip php client
1388f21
@iammac360
Implement fetch_tickers(), fetch_ticker(), fetch_order_book(), fetch_…
442f0ea 
…ohlcv() and fetch_trades() methods for coinzip php client
@iammac360
Implement fetch_balance(), fetch_order(), fetch_orders(), fetch_open_…
b58600a 
…orders(), fetch_closed_orders(), create_order() and cancel_order() methods for coinzip php client
@iammac360
Implement fetch_my_trades() method for coinzip php client
a460894
@iammac360
Refactor coinzip js client
d2aa515
@iammac360
Change api and www url
0947e38
@iammac360
Fix fetchTicker and fetchTicker responses
212e0b3
@iammac360
Refactor fetchTickers and fetchOHLCV
a6436b6
@iammac360
Fix api url
06aa6ea
@iammac360
Fix documents url
7673753
@iammac360
Update logo url
5e99751
@iammac360
Fix urls
7961860
@iammac360
Add coinzip for browser build
860ac3e
@iammac360
Add python3 support for coinzip
7f2e42f
@iammac360
Fix failing tests for php
5a74138
@iammac360
Refactor coinzip js
04c7043
@iammac360
Run npm build and fix failing tests
fae1995
@iammac360
Reverted generated files to master
7fa1c74
@iammac360
Deleted coinzip.php and coinzip.py files. This will be generated …
1b5e256 
…on Travis CI
@iammac360
Copy link
Author

Hi @kroitor any feedback?

@kroitor
Copy link
Member
kroitor commented Apr 13, 2019

@iammac360 pardon for the long wait, i'll do my best to get to it asap, didn't have the chance to do it yet, hope for your understanding.

@ttodua ttodua changed the title Add a new exchange: Coinzip New Exchange: Coinzip Jun 10, 2024
@ttodua
Copy link
Member
ttodua commented Jun 10, 2024

dead exchange

@ttodua ttodua closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kroitor kroitor

Labels
new exchange
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iammac360 @kroitor @ttodua
0