Skip to content

Try to decrease dependencies#211

Open
elichai wants to merge 1 commit into
koute:masterfrom
elichai:2019-08-slim-deps
Open

Try to decrease dependencies#211
elichai wants to merge 1 commit into
koute:masterfrom
elichai:2019-08-slim-deps

Conversation

@elichai

@elichai elichai commented Aug 12, 2019

Copy link
Copy Markdown
Contributor

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

koute commented Aug 13, 2019

Copy link
Copy Markdown
Owner

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

elichai commented Aug 13, 2019

Copy link
Copy Markdown
Contributor Author

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

elichai commented Aug 13, 2019

Copy link
Copy Markdown
Contributor Author

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

elichai commented Aug 14, 2019

Copy link
Copy Markdown
Contributor Author

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

koute commented Aug 20, 2019

Copy link
Copy Markdown
Owner

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