-
-
Notifications
You must be signed in to change notification settings - Fork 97
clean up dependency tree #338
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
Conversation
e1a5f71 to
5bf68a5
Compare
|
Thanks for this - overall looks good. At some point in time I had left |
|
the 1.81 msrv is mostly due to the |
5bf68a5 to
688da71
Compare
|
one more note: i realized that |
|
Thanks for the investigation there. I think, if possible, I'd like to try to keep the MSRV for steel core on 1.76 if possible. If its not trivially fixable with the home changes you've already made + removing the commit for the the once cell stuff, then we can merge this as is. |
688da71 to
75b6bbe
Compare
|
done now. i used the
|
75b6bbe to
3d70616
Compare
|
tbe ci is failing because something on windows is trying to use cmake version less than 3.5. what. i have no idea how to even attempt to fix that lol sorry |
|
I saw that happening yesterday, I'll take a look, don't worry about it |
|
I'm happy with this as is - would you be willing to fix the merge conflicts? If not, I can fix them 😃 |
3d70616 to
d1aada2
Compare
|
rebased and fixed the merge conflicts |
d1aada2 to
84097d7
Compare
|
rebased again and fixed the other merge conflicts from #360 |
|
agh, another PR just introduced a small merge conflict This is the next PR to go in - once the conflicts are fixed I'll merge so you don't have to fight them again |
i think it's better style to activate the "derive" feature on rustyline.
just use the needed dependencies directly
84097d7 to
2e59266
Compare
|
done again. also don't worry about it, my fault for doing a pretty big pr and i'd rather me do the merge conflicts than have other people have to worry about it (also most of the merge conflicts here were introduced by other prs from me) |
this pull request is purely a clean up for your dependency tree and should not change any of the features. i partially used cargo-machete for the low-hanging fruit (which is not super accurate) or just looked at the output from
cargo-treeand kind of guessed and then looked if i can remove any of the dependencies.[64364e1]: thethis is actually no longer possible since don't exit the repl when ctrl-c-ing on a non-empty line #349. whoops.rustylinecrate by default enables a feature for custom keybinds (custom-bindings) which steel doesn't actually need and this was pulling in a few unnecessary dependenciesserde_deriveas you were already dependingserdewith thederivefeature, and remove theprettydependency. this doesn't actually change much sinceserdedepends onserde_deriveandsteel-parserdepends onpretty, but i think this makes the dependency graph a little cleaner.dirsandhomedependency fromsteel-core. steel was already pulling in theenv_homecrate throughwhichand thedirsdependency was actually only used for getting the user home directory, whichenv_homecan do too. if your msrv ever goes above 1.87 (the current nightly) you can actually just usestd::env::home_dir()as that was very recently undeprecated (Undeprecate env::home_dir rust-lang/rust#137327).serde_derive, just in all the other crates.serdefrom steel-interpreter. as it was a dev dependency, this doesn't really affect anything, but it wasn't needed.once_cellwith thestdequivalents, asstd::sync::LazyLockwas stabilized only in 1.80 and steel's msrv is 1.76, but i can replace it for thesteel-language-server.coloredcrate to get rid oflazy_static. sincecoloredwas only used by thesteel-replwhich has an msrv of 1.81 anyway due to rustyline, it should be fine that colored v3 has an msrv of 1.80.steel-corewas dependending on thehttpcrate, but wasn't actually using it.Cargo.tomls about not pulling in the entirety ofnumand only depending on what was actually needed. this commit does exactly that: it replaces thenumdependency and directly relies onnum-traits,num-rationalandnum-bigintnumdependency, but instead withcrossbeamwhere you only neededcrossbeam-channelandcrossbeam-utils. this change doesn't actually fully removecrossbeamfrom the dependency tree, as it is still used bytermimad, but i opened a pr there as well: breaking: directly depend on crossbeam-channel instead of pulling in all of crossbeam Canop/termimad#69.env_loggerdev dependency in steel-core.none of these changes are "make-or-break" to me and i would be willing to drop any of the changes if you prefer not to have them.