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

Try to decrease dependencies #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elichai
Copy link
Contributor

@elichai elichai commented Aug 12, 2019

The time it takes to compile this library is getting pretty long. especially on CIs.
This is a naive start to hopefully decrease compile time.
here I just disabled unused features in dependencies(just this removed around 80 lines in the Cargo.lock)

@koute
Copy link
Owner

koute commented Aug 13, 2019

Thanks for the PR!

  1. From what I can see on Travis it looks like one of your changes might have broken compilation without a Cargo.lock file. (See the job which sets WITHOUT_CARGO_LOCK=1.) Could you please check it out?
  2. Please split/squash your changes into two commits - one which changes the Cargo.toml, and one which changes the Cargo.lock. (In general I like to keep the Cargo.lock changes in a separate commit as later it's easier to browse through history, etc.) Besides this it looks good to me as long as (1) will be fixed!

@elichai
Copy link
Contributor Author

elichai commented Aug 13, 2019

FYI it seems weird to use danger_accept_invalid_certs where it could be a HTTPS_PROXY (in general that's a weird function)

@elichai
Copy link
Contributor Author

elichai commented Aug 13, 2019

Ok, it updated reqwests to a new version, so I had to update the whole Cargo.lock

Updating the cargo.lock without this PR results in:
1 file changed, 1090 insertions(+), 778 deletions(-)

With this PR:
1 file changed, 1010 insertions(+), 855 deletions(-)

Not a massive change, but a change non the less. (80 less insertions and 77 more deletions)

@elichai
Copy link
Contributor Author

elichai commented Aug 14, 2019

Seems like travis fails on cargo check --target=x86_64-apple-darwin
probably new version of backtrace-sys requires some system library that isn't there, why don't you run this test on osx?

@koute
Copy link
Owner

koute commented Aug 20, 2019

Ugh, it looks like a C compiler is necessary now while it wasn't necessary before; backtrace-sys really ought to not require a full cross toolchain just to run cargo check. -_-

But you have a good point. I've just removed the checks and just put them in their own dedicated jobs for OS X and Windows. We'll see how it goes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants