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

Add machete and remove cargo-udeps #5

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Conversation

tiruka
Copy link
Contributor

@tiruka tiruka commented Sep 8, 2024

Related Issues

#3

Checks

Looks like test validation process is not established for this repo, I did following checks. If not enough, pleas let me know how to deal with additional checks and tests.

No Detection Case

After building, run the command with execution files in the target dir.
As expected no errors occurred.

% cargo build                                   
% ./target/xtask/debug/xtask dependencies unused
[2024-09-08T12:23:34Z INFO  tracel_xtask] Execution environment: std
[2024-09-08T12:23:34Z INFO  tracel_xtask::commands::dependencies] Cargo: run unused dependencies checks
[2024-09-08T12:23:34Z INFO  tracel_xtask::utils::process] Command line: cargo machete
Analyzing dependencies of crates in this directory...
cargo-machete didn't find any unused dependencies in .. Good job!
Done!

Detection Case

To detect unused packages, I added cfg-if for instances and executed the command.

% cargo add cfg-if                    
error: `cargo add` could not determine which package to modify. Use the `--package` option to specify a package. 
available packages: tracel-xtask, tracel-xtask-macros, xtask
toru@toru-mac xtask % cargo add cfg-if --package tracel-xtask
    Updating crates.io index
      Adding cfg-if v1.0.0 to dependencies
             Features:
             - compiler_builtins
             - core
             - rustc-dep-of-std

After building, run the command with execution files in the target dir.
As expected an error occurred.

% cargo build   
% ./target/xtask/debug/xtask dependencies unused
[2024-09-08T12:24:21Z INFO  tracel_xtask] Execution environment: std
[2024-09-08T12:24:21Z INFO  tracel_xtask::commands::dependencies] Cargo: run unused dependencies checks
[2024-09-08T12:24:21Z INFO  tracel_xtask::utils::process] Command line: cargo machete
Analyzing dependencies of crates in this directory...
cargo-machete found the following unused dependencies in .:
tracel-xtask -- ./crates/tracel-xtask/Cargo.toml:
        cfg-if

If you believe cargo-machete has detected an unused dependency incorrectly,
you can add the dependency to the list of dependencies to ignore in the
`[package.metadata.cargo-machete]` section of the appropriate Cargo.toml.
For example:

[package.metadata.cargo-machete]
ignored = ["prost"]

You can also try running it with the `--with-metadata` flag for better accuracy,
though this may modify your Cargo.lock files.

Done!
Error: Unused dependencies found!

@tiruka
Copy link
Contributor Author

tiruka commented Sep 8, 2024

Conflicts is detected for Cargo.lock. It has changed due to my editor (VSCode) automatically, so it may be correct. If downgrade is necessary, I will do that with vim or something else.

endgroup, group,
utils::{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary, so delete them.

Copy link

@Luni-4 Luni-4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your work! It's fine for me!

If @syl20bnr agrees, we can merge this PR

Copy link
Member

@syl20bnr syl20bnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Thank you !

@syl20bnr syl20bnr merged commit fb21914 into tracel-ai:main Sep 19, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants