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

uucore: put more deps behind feature flags (for nushell) #5202

Closed
tertsdiepraam opened this issue Aug 24, 2023 · 4 comments
Closed

uucore: put more deps behind feature flags (for nushell) #5202

tertsdiepraam opened this issue Aug 24, 2023 · 4 comments

Comments

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Aug 24, 2023

Context: nushell/nushell#10097 (comment)

uucore is quite modular in that large parts can be enabled or disabled via features. However, this does go far enough yet. There are many modules in the mods folder, that should be put behind feature flags.

This would make it so that uucore becomes smaller and faster to compile when only compiling a subset of utils. This is of particular interest for nushell, where we are introducing just a single util (cp), but have to include the entirety of uucore.

Fixing this should not be too hard. However, we should pay special attention to verify that every util still compiles on its own. That is, that they individually enable all the features they need. Every util should compile like this:

cargo build --no-default-features --features {util}

Ideally, we would also test this in the CI. See also #4032

PS: Ultimately, I think uucore should be split into multiple crates, but that's an issue for another time. If we make it more modular now, we can also more easily split it up in the future.

@cakebaker
Copy link
Contributor

Is there more work necessary for this ticket to be considered as "done" or can I close it?

@sylvestre
Copy link
Sponsor Contributor

is it possible to add a check in the CI to make sure we don't regress?

@cakebaker
Copy link
Contributor

@sylvestre there's #4032 or do you mean something else?

@tertsdiepraam
Copy link
Member Author

@cakebaker I'm happy with this! The only thing "missing" is that clap is still required, but that is a larger issue and covered by the "compile cp without clap" issue. Thanks for working on this!

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

No branches or pull requests

3 participants