wisespace-io / binance-rs

Rust Library for the Binance API
Other
671 stars 300 forks source link

Add more unit tests #96

Closed ppamorim closed 3 years ago

ppamorim commented 3 years ago

In this PR:

dorak88783 commented 3 years ago

Very very nice, impressive amount of work.

ppamorim commented 3 years ago

@dorak88783 I am fighting with the compiler, cargo build in my machine does not compile the doctest by some reason...

dorak88783 commented 3 years ago

@dorak88783 I am fighting with the compiler, cargo build in my machine does not compile the doctest by some reason...

you should be able to see exactly what github is doing by looking at the config in .github/action; if you run exactly the same on your machine then there's some other configuration option. Do you have the latest version of rust?

I never worked with doctest myself.

dorak88783 commented 3 years ago

@ppamorim I'm only not really sold on the change from () to the custom-made Void. I thought Void was a built-in but it's something custom. I don't have enough Rust experience to really judge this.

ppamorim commented 3 years ago

@ppamorim I'm only not really sold on the change from () to the custom-made Void. I thought Void was a built-in but it's something custom. I don't have enough Rust experience to really judge this.

() causes serde to crash.

ppamorim commented 3 years ago

@dorak88783 I am fighting with the compiler, cargo build in my machine does not compile the doctest by some reason...

you should be able to see exactly what github is doing by looking at the config in .github/action; if you run exactly the same on your machine then there's some other configuration option. Do you have the latest version of rust?

I never worked with doctest myself.

It's the same strangely or not. Anyway it's fixed.

wisespace-io commented 3 years ago

@ppamorim Great work here.

ppamorim commented 3 years ago

@ppamorim Great work here.

Thank you very very much!

Would you like to rename Void to something else? I am not sure how to name it properly.

wisespace-io commented 3 years ago

@ppamorim Great work here.

Thank you very very much!

Would you like to rename Void to something else? I am not sure how to name it properly.

Is it just an empty struct? You may use a synonym (adjective) such as Emptied, Blank, Unfilled

dorak88783 commented 3 years ago

@ppamorim I now see there's also a create called void, maybe this also provides a more uniform/standard way to use this Void? I didn't check it myself yet. https://docs.rs/void/1.0.2/void

ppamorim commented 3 years ago

@wisespace-io @dorak88783 I think there is a way to map Void (internally it will be called Empty) to () which makes more sense and it won't break the code.

ppamorim commented 3 years ago

.map(|empty| ()) did the job.