-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore: cleanup workspace crates #8058
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
* properly add all crates to the workspace cargo.toml as members * except `fuzz` because it still has some issues, TBD * use quotes around `true` and `false` to ensure there is no bool confusion * remove a few leftover readme comments * mark all unpublishable crates as `publish = false` to avoid accidental publishing * Add `uutests` to the main workspace
|
GNU testsuite comparison: |
Cargo.toml
Outdated
| xattr = { workspace = true } | ||
|
|
||
| # Specifically used in test_uptime::test_uptime_with_file_containing_valid_boot_time_utmpx_record | ||
| # Specifically, used in test_uptime::test_uptime_with_file_containing_valid_boot_time_utmpx_record |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see why you would add a comma here, but I think that's not quite right. It should be read more as in "Used specifically in ..."
Cargo.toml
Outdated
| expr = { optional = true, version = "0.1.0", package = "uu_expr", path = "src/uu/expr" } | ||
| factor = { optional = true, version = "0.1.0", package = "uu_factor", path = "src/uu/factor" } | ||
| false = { optional = true, version = "0.1.0", package = "uu_false", path = "src/uu/false" } | ||
| "false" = { optional = true, version = "0.1.0", package = "uu_false", path = "src/uu/false" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it worked before right? Are toml keys not guaranteed to be strings? I think they should be according to https://toml.io/en/v1.0.0#keys
|
Nice cleanup! |
|
@tertsdiepraam I modified the comment (there was another minor grammmar nit) - hope its better now. WRT the keys - it did work before, but IntelliJ IDEA was flagging it (incorrectly) - so this was to placate it. I am ok to revert that if you think it is not needed. |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
As noticed by @cakebaker in #8056 (comment), |
|
GNU testsuite comparison: |
| "src/uucore", | ||
| "src/uucore_procs", | ||
| "src/uuhelp_parser", | ||
| "tests/benches/factor", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is very rarely used, why list it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this forces all crates to still be part of the regular compilation / validation / linting, use the same dependencies, gets regular updates, etc. I.e. prevent bitrot - unless it is not needed and should be deleted?
fuzzbecause it still has some issues, TBDtrueandfalseto ensure there is no bool confusionpublish = falseto avoid accidental publishinguuteststo the main workspacetests/benches/factor