Remove unused dependencies #1976
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
No milestone
No assignees
7 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
continuwuation/continuwuity!1976
Loading…
Reference in a new issue
No description provided.
Delete branch "lake/continuwuity:feature/shear-unused-deps"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This pull request:
cargo-shear] (and configures it to ignore the dependencies it flagged as false positives)cargo shearcheck to CI to make sure that if any new unused dependencies will be automatically caught in the futureThe final count reported by
cargo-shear(after ignoring false positives) was 19 (edit: now likely fewer) removed dependencies for the workspace (this number is not unique, ascargo-shearcounts the workspace and each crate separately).Closes #1964.
Note:
cargo-shearalso reported some unused files, and dependencies that are only used in features, but not marked as optional, with a combined 19 warnings. I may try to address those in a future PR.Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
WIP: Remove unused dependenciesto Remove unused dependencies416d5ca4e6550a5c68f7550a5c68f7c1ecc18572c1ecc18572eb885f9486Remove unused dependenciesto WIP: Remove unused dependencieseb885f9486b74b238e0aWIP: Remove unused dependenciesto Remove unused dependencies@ -47,7 +44,6 @@ tower-sessions-core = { version = "0.15.0", features = ["deletion-task"] }serde_urlencoded.workspace = trueurl.workspace = truerecaptcha-verify = { version = "0.2.0", default-features = false }reqwest_recaptcha = { package = "reqwest", version = "0.12.28", default-features = false, features = ["rustls-tls-native-roots-no-provider"] } # As long as recaptcha-verify's reqwest is outdatedThis is needed to explicitly enable the TLS backend feature, because recaptcha-verify doesn’t expose the option we want and uses an outdated version so it isn't enabled by another crate
missclick
b74b238e0a90ba751a2fThe irony is this doesn't actually cause any dependencies to be removed from the lockfile lol
@Jade wrote in #1976 (comment):
hm, that's not what I'm seeing. I did squash the lockfile, which caused a force push, so maybe new changes weren't visible at first
@ -11,7 +11,6 @@ version.workspace = true[dependencies]conduwuit-admin.workspace = trueconduwuit.workspace = trueThis is also intentional, it is supposed to make sure that there is no thrashing due to changes in the enabled features in dependencies. I haven't checked that this works as intended though
do you want me to restore it?
There are deleted lines from the lockfile, however other crates depend on each of these crate versions, so no crate version is fully removed. going by this version
It's a good idea to do anyway though.
@Jade wrote in #1976 (comment):
got it, fair enough. though it still removes those dependencies for those crates
As long as this builds properly I have no major objections, only the few nitpicks in the other comments
@ -0,0 +1 @@Removed unused dependencies and added cargo-shear step to CI. Contributed by Lake (@ezeraOC)Unsure if
featureis the correct name here, as it doesn't introduce anything new for end-users. I'd lean more towardsmisc@ -10,0 +6,4 @@name = "conduwuit_web"readme.workspace = truerepository.workspace = trueversion.workspace = trueAs much as i love a well-formatted file, the formatting here seems out-of-scope for this PR? Can also make it more difficult to find the original creator of a line through
git blameit's now pulled out in a separate commit and ignored in
.git-blame-ignore-revs. if you really don't want it here at all, I can drop the commit.Works for me :)
90ba751a2f862ce0f67d862ce0f67d7c17fe409c7c17fe409ca0f8c818f3lgtm
Looks like this has merge conflicts, could it be rebased?
@Henry-Hiles wrote in #1976 (comment):
done!
to avoid this falling behind again, it should be merged soon. any ongoing dependency/code churn in c10y essentially requires repeating the loop anew (resolve conflicts -> build -> run tests -> shear to catch anything that became unused since the last time -> catch shear's false positives -> build+test again), which gets fiddly.
e1738667a28ab842b198Builds and runs, LGTM.
This works and runs. I have webpush merged in my branch and it required this to pass clippy/build/etc