wisespace-io / binance-rs

Rust Library for the Binance API
Other
640 stars 290 forks source link

Fixing typo for Spot::Ticker24hr in api.rs #98

Closed nopbit closed 3 years ago

nopbit commented 3 years ago

Added "r" letter at the end of api address. With this fix, it is not giving 404 error anymore.

dorak88783 commented 3 years ago

Thanks for noticing and opening the PR!

Your build currently fails because the test code also has the same typo. If you could also fix the typo in https://github.com/wisespace-io/binance-rs/blob/master/tests/market_tests.rs , then likely the build will work.

nopbit commented 3 years ago

Thanks, i updated test file also and merged the patches. Best regards

dorak88783 commented 3 years ago

@wisespace-io It seems you now have to manually approve the workflow for new contributors to it will do the cargo build & test.

nopbit commented 3 years ago

I wanted to merge my patch-1 and patch-2 but it is now saying " 1 workflow awaiting approval First-time contributors need a maintainer to approve running workflows. Learn more. " I can close this request and create new one.

wisespace-io commented 3 years ago

@dorak88783 @nopbit Sorry, for the delay. I approved the workflow. Anything else missing?

dorak88783 commented 3 years ago

Ah, there are two test cases that refer to this end point. It fails in get_all_24h_price_stats, this one also needs to be updated.

nopbit commented 3 years ago

To be honest i have no idea why it is giving error on test. i will check it on local copy

dorak88783 commented 3 years ago

Search https://github.com/wisespace-io/binance-rs/blob/master/tests/market_tests.rs for /api/v3/ticker/24h, you'll find two locations, and you already fixed one.

You can indeed run cargo test locally, it should pass before you push your commits.

nopbit commented 3 years ago

Thanks, this time test passed on local copy. Could you please approve it?