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

Reduce dependencies #62

Merged
merged 2 commits into from
Feb 13, 2021
Merged

Conversation

CosmicHorrorDev
Copy link
Collaborator

@CosmicHorrorDev CosmicHorrorDev commented Feb 12, 2021

Description

Contributions

  • I changed the dependency listing because it reduces the compile times significantly

Three of the dependencies were not used in the project (pad, dialoguer, and regex) and sysinfo pulls in rayon which has a significant amount of dependencies and I don't believe that this library was getting significant use from it (the changes are discussed here GuillaumeGomez/sysinfo#382). This reduced the number of crates from 112 to 70 and also significantly reduced compile times.

I didn't test the docker image locally, but cargo test came up with no problems. I'll test the changes in a docker container later today although I don't foresee any issues.

Checklist

  • I added one or multiple labels which best describes this PR.
  • I have tested the changes locally.
  • This PR has a reviewer on it.
  • I have validated my changes in a docker container and on Ubuntu. (Only needed for Odin or Docker Changes)

@mbround18 mbround18 added enhancement New feature or request odin Tag if theres an issue with odin labels Feb 13, 2021
@mbround18 mbround18 self-requested a review February 13, 2021 00:24
@mbround18
Copy link
Owner

This looks good! I am fairly new to rust and this is excellent! I did not know I could strip deps like that.

@mbround18 mbround18 merged commit caacf3f into mbround18:main Feb 13, 2021
@CosmicHorrorDev
Copy link
Collaborator Author

The project seems pretty well organized for you being fairly new!

And I meant to mention it in the original PR (was actually distracted playing valheim while drafting it up), but the tool I used for finding unused dependencies was the cargo extension cargo udeps (rustup run nightly cargo udeps since it requires nightly). Other than that it was just using cargo tree (another cargo extension) to find what pulls in a lot of dependencies and then poking around at the features of those crates to see what can be slimmed down (going to the doc.rs page and looking under the feature flags).

And I'm definitely planning on contributing more to this project if you're fine with that! This was just the entry PR so that developing on my very old laptop is less painful 😅

@mbround18
Copy link
Owner

Thank you <3 I appreciate the links and the compliment :) I try to make code easy to understand for others to look at. Absolutely! I always welcome pull requests! and this one was an amazing one! It gave a 5 and a half minute reduction on build+deploy times!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request odin Tag if theres an issue with odin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants