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

Prune unused deps + general dependency cleanup #324

Merged
merged 3 commits into from
Apr 11, 2023

Conversation

actioninja
Copy link
Contributor

@actioninja actioninja commented Apr 6, 2023

I'm submitting a

  • bug fix (?)
  • feature
  • documentation addition

All dependency entries were sorted.

eval, fs_extra, and all dev dependencies were dropped from the app crate as they were unused. eval, fs_extra, openssl, paw, structopt, andtracing-subscriber were dropped from regular dependencies of the lib crate, and tempdir, assert_cmd, predicates, and assert_fs were removed from dev-dependencies.

In one test, usage of tempdir was replaced with the tempdir functionality of tempfile as tempdir is deprecated.

Every dependency was bumped to the latest possible minor version. Major versions would likely require a more hand-done audit, but minor versions are supposed to be back-compatible and safe to update aggressively. Tests still run fine after doing this, so I'm erring on that semver was properly followed for dependencies.

The only potentially contentious part here is that I dropped the patch version pin on each dep, preferring instead to only specify major.minor. The whole dep tree is already pinned via Cargo.lock, and I prefer to let the most recent patch be more aggressively consumed. Due to the way cargo resolves dependencies, Minor version is much less safe of a guarantee due to the way that Minor doesn't need to be forward-compatible by design.

If there's a particular reason why every dep is pinned down to patch version, I'll go through and pin each to the current latest patch for the minor version. Patch version changes should always be cross compatible both forward and back, the exception here would be non-semver compliant crates.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (a92abf2) 44.96% compared to head (001d500) 44.96%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #324   +/-   ##
=======================================
  Coverage   44.96%   44.96%           
=======================================
  Files          82       82           
  Lines        2942     2942           
=======================================
  Hits         1323     1323           
  Misses       1619     1619           
Impacted Files Coverage Δ
lib/src/atoms/file/copy.rs 81.35% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@icepuma
Copy link
Member

icepuma commented Apr 6, 2023

Na, there is no particular reason they should be pinned. Major.Minor should be enough.

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

4 participants