wisespace-io / binance-rs

Rust Library for the Binance API
Other
649 stars 295 forks source link

Cleanup API #84

Closed ppamorim closed 3 years ago

ppamorim commented 3 years ago

I did some cleanup of the API to make it more concise and easy to maintain.

I also removed unnecessary conversion from Data -> String -> JSON, now the library converts Data -> JSON directly and the code got simplified.

This PR will be opened when #83 gets merged.

ppamorim commented 3 years ago

@dorak88783 Missed to fix the general file.

ppamorim commented 3 years ago

@dorak88783 Could you please verify if all routes are correct?

dorak88783 commented 3 years ago

@ppamorim I can try, but likely only next week. I was also working on creating end-to-end test cases using the mockito package, but need more time to get this into shape for a PR.

dorak88783 commented 3 years ago

I had a quick scan and the endpoints look okay to me. Maybe you could sort them such that they appear in the order as in the API doc at https://github.com/binance/binance-spot-api-docs/blob/master/rest-api.md; and make a comment for the ones that are not implemented yet. I can also do this later in a new PR.

ppamorim commented 3 years ago

@dorak88783 All done :)

wisespace-io commented 3 years ago

@ppamorim I pulled your branch and noticed some duplicated definitions. You can check the errors in https://travis-ci.org/github/wisespace-io/binance-rs ... also there are some clippy errors, you can also check in travis, or just run "cargo clippy -- -D warnings"

ppamorim commented 3 years ago

@wisespace-io This happened because draft PR #85 was merged before this PR. I create a PR to revert the changes. I also sorted the issues in this PR.

Please apply the revert and after review this PR.

ppamorim commented 3 years ago

@wisespace-io Build passed :) https://travis-ci.org/github/wisespace-io/binance-rs/builds/765038918

wisespace-io commented 3 years ago

@dorak88783 have you finish the review? @ppamorim fixed the build and clippy warnings.

dorak88783 commented 3 years ago

@wisespace-io : I'm honored to be asked to review :), but I don't think I'm a critical reviewer. I can have a look again but only on Thursday; I think it's fine to merge like this.

ppamorim commented 3 years ago

@dorak88783 Any update?

dorak88783 commented 3 years ago

Thanks for checking, but didn't have time today. Maybe in the weekend, but I'd not wait for me :)