-
-
Notifications
You must be signed in to change notification settings - Fork 85
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-tree
and 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.rustyline
crate 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_derive
as you were already dependingserde
with thederive
feature, and remove thepretty
dependency. this doesn't actually change much sinceserde
depends onserde_derive
andsteel-parser
depends onpretty
, but i think this makes the dependency graph a little cleaner.dirs
andhome
dependency fromsteel-core
. steel was already pulling in theenv_home
crate throughwhich
and thedirs
dependency was actually only used for getting the user home directory, whichenv_home
can 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.serde
from steel-interpreter. as it was a dev dependency, this doesn't really affect anything, but it wasn't needed.once_cell
with thestd
equivalents, asstd::sync::LazyLock
was stabilized only in 1.80 and steel's msrv is 1.76, but i can replace it for thesteel-language-server
.colored
crate to get rid oflazy_static
. sincecolored
was only used by thesteel-repl
which has an msrv of 1.81 anyway due to rustyline, it should be fine that colored v3 has an msrv of 1.80.steel-core
was dependending on thehttp
crate, but wasn't actually using it.Cargo.toml
s about not pulling in the entirety ofnum
and only depending on what was actually needed. this commit does exactly that: it replaces thenum
dependency and directly relies onnum-traits
,num-rational
andnum-bigint
num
dependency, but instead withcrossbeam
where you only neededcrossbeam-channel
andcrossbeam-utils
. this change doesn't actually fully removecrossbeam
from 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_logger
dev 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.