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

chore: remove unused things #63

Merged
merged 1 commit into from
Mar 13, 2024
Merged

chore: remove unused things #63

merged 1 commit into from
Mar 13, 2024

Conversation

helio-frota
Copy link
Collaborator

@helio-frota helio-frota commented Mar 12, 2024

  • focusing on graph dir only

Related to #17

@helio-frota helio-frota marked this pull request as ready for review March 12, 2024 19:53
Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

This is doing a lot more than just "removing unused things". Let's start by just removing all the unused imports.

I'm curious, though: what's the reason for all the #[allow(dead_code)] and TODO comments?

@helio-frota
Copy link
Collaborator Author

helio-frota commented Mar 12, 2024

This is doing a lot more than just "removing unused things". Let's start by just removing all the unused imports.

I agree. I'm using cargo check and, when removing the imports + removing #![allow(unused)] it started to complain about good amount of things.

I'm curious, though: what's the reason for all the #[allow(dead_code)] and TODO comments?

Good question 👍

Due to the experimental appearance of the code (in some parts), I preferred to use dead_code for struct attributes and _ for unused function arguments, and "TODOs" to review later and check if we really need the variables that starts like this _foo

All these points appeared after I removed the imports and #![allow(unused)]

➜ trustify git:(unused) rg "\#\!\[allow\(unused\)\]"
server/src/lib.rs
1:#![allow(unused)]

common/src/lib.rs
1:#![allow(unused)]

@helio-frota
Copy link
Collaborator Author

Reading again the issue I got the thing... we are not kind of prepared to remove this part #![allow(unused)] yet...

I'll add back #![allow(unused)] and revert the other changes 👍

* focusing on graph dir only
@helio-frota
Copy link
Collaborator Author

@jcrossley3 PR updated 👍

Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

Thank you!

@helio-frota
Copy link
Collaborator Author

@jcrossley3 thanks!

@helio-frota helio-frota merged commit cbf25b5 into trustification:main Mar 13, 2024
1 check passed
@helio-frota helio-frota deleted the unused branch March 13, 2024 13:30
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.

2 participants