Skip to content
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.

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

Are all dependencies really needed? #12

Open
auterium opened this issue Jul 17, 2020 · 3 comments
Open

Are all dependencies really needed? #12

auterium opened this issue Jul 17, 2020 · 3 comments

Comments

@auterium
Copy link

Hello there :)

I started looking at the code and something quickly caught my eye: there seem to be room for improvement in the dependencies.

Chrono

This is included in the coinbase sub-crate but never used in the code. I see it's used in binance sub-crate only to get the timestamp in milliseconds. This can be achieved also with the std library:

use std::time::SystemTime;

fn main() {
    let ts = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_millis();
    println!("{}", ts);
}

Reqwest: blocking

Using reqwest with the blocking feature will block the entire thread while waiting for the response from the server. This can kill the performance of the dependent project. I've experienced the complexity of using the async version of reqwest on earlier versions, but seems it's working much better in the latest version.

Tokio: full

This pulls all of tokio while only a few parts are needed in the compilable code, while the others can be moved to [dev-dependencies]

Dotenv

Love the crate, but this is only required for the tests, so why not have it [dev-dependencies]?

Ring

Can't find where it's used 😢

I'd be more than happy to help on improving and building this library. I can do a PR with some of the proposed cleanups if you like.

Regards,
Marc

@ghost
Copy link

ghost commented Jul 20, 2020

Thanks Mark,
The dependencies are for sure improvable for sure as I have been moving things around and from time to time forgot to remove the unused.

Your PR is welcomed!

@TimvanScherpenzeel
Copy link

ring was originally used for the Hmac implementation but that has been replaced by the hmac and sha2 so it can be safely removed.

@ghost
Copy link

ghost commented Aug 24, 2020

#58

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

No branches or pull requests

2 participants