Skip to content

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 elichai force-pushed the 2019-08-slim-deps branch from 1ad8bae to ab063c6 Compare August 13, 2019 22:17
@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.

2 participants