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.

Sign up for GitHub

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

Jump to bottom

build(linting): add more lints #537

Merged
merged 17 commits into from
Aug 6, 2024
Merged

build(linting): add more lints #537

merged 17 commits into from
Aug 6, 2024

Conversation

Rudxain
Copy link
Member

@Rudxain Rudxain commented Jul 5, 2024 •
edited
Loading

@Rudxain Rudxain added debug testing Validation and tests and removed debug labels Jul 5, 2024
@deepsource-io deepsource.io
Copy link
Contributor

deepsource-io bot commented Jul 5, 2024 •
edited
Loading

Here's the code health analysis summary for commits 55833e9..0209d28. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Rust LogoRust❌ Failure
❗ 1 occurence introduced
View Check ↗
DeepSource Test coverage LogoTest coverage⚠️ Artifact not reportedTimed out: Artifact was never reportedView Check ↗
💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@AnonymousWP
Copy link
Member

Please rework the title of this PR as it's too generic. Saying this because the changelog content will be based on the title of PRs.

@Frigyes06
Copy link
Member

Nooo, now we'll have to do proper result handling and such :(
No more bad code

@Rudxain Rudxain changed the title build(lints): add more build(linting): add more lints Jul 8, 2024
@Rudxain
Copy link
Member Author

Rudxain commented Jul 8, 2024 •
edited
Loading

Nooo, now we'll have to do proper result handling and such :(

lol 💀😅. We can still use expect! My reasoning for unwrap_used is that it'll make debugging easier (at the cost of slower prototyping)

@Frigyes06
Copy link
Member

Should the actual fixes to the issues uncovered by the lints be in this PR or a separate PR?

@Rudxain
Copy link
Member Author

Rudxain commented Jul 11, 2024 •
edited
Loading

Should the actual fixes to the issues uncovered by the lints be in this PR or a separate PR?

I was considering the same thing 🤔

This PR should be for deciding which lints to add and which to allow (turn off)

After the decision is made, we can gradually apply the fixes in a separate PR

@Rudxain
Copy link
Member Author

Rudxain commented Jul 11, 2024

Should we warn on pedantic group? I'm too lazy to find individual lints to enable, lol. So we could simply allow individual lints that are annoying, and keep the rest

@Rudxain Rudxain marked this pull request as ready for review July 11, 2024 18:31
@Rudxain Rudxain requested a review from Frigyes06 July 11, 2024 18:31
@AnonymousWP AnonymousWP requested review from a team and removed request for Frigyes06 July 12, 2024 16:11
Rudxain
Rudxain commented Jul 12, 2024
View reviewed changes
Cargo.toml Show resolved Hide resolved
@AnonymousWP
Copy link
Member

@Universal-Debloater-Alliance/developers Please pick this up. See comments/questions above.

@Rudxain
Copy link
Member Author

Rudxain commented Aug 6, 2024

I'll merge when I have time (probably in ~3 hours) to fix the warnings in a separate PR

@Rudxain Rudxain merged commit 413634e into main Aug 6, 2024
7 of 12 checks passed
@Rudxain Rudxain deleted the build/add-lints branch August 6, 2024 20:28
@Rudxain Rudxain mentioned this pull request Aug 6, 2024
Merged
@Rudxain
Copy link
Member Author

Rudxain commented Aug 7, 2024 •
edited
Loading

When Rust 1.81 releases on stable, we should consider this

AnonymousWP pushed a commit that referenced this pull request Sep 14, 2024
@Rudxain @deepsource-autofix
refactor(linting): fix warnings from #537 (#578)
b50d7af 
* refactor(apply-lints): `cargo clippy --fix`

* style: format code with Rustfmt

This commit fixes the style issues introduced in 7c04d15 according to the output
from Rustfmt.

Details: #578

* style(save): call unqualified `Ok`

* style(sync): inline fmt args on `info!`

* style(theme): properly separate hex colors, `allow` `match_same_arms`

* refactor(list-view): `allow` some lints, dedupe text widget

* refactor(list-view): fix 1 `if_not_else`

* fix(utils): `setup_uad_dir` no longer takes `Option`

Also more logging and error handling

* refactor(list-view): `export_selection` `Button` built with common branch

* fix(list-view): fix all warns in 1 file, refactor, err handling, etc...

* style: format code with Rustfmt

This commit fixes the style issues introduced in 49cfa27 according to the output
from Rustfmt.

Details: #578

* refactor(lints): `allow`, `expect`, and apply lints. factor `uad_lists.json` into var
Also:
- factors-out the List name into a `const`.
- Converts some regular comments into docs.
- use `reason` as a better alternative to comments.

* docs(name): concise description of `NAME`

* style(theme): apply suggestion from review

---------

Co-authored-by: Rudxain <rudxain@localhost.localdomain>
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lavafroth lavafroth lavafroth approved these changes

Assignees
No one assigned
Labels
testing Validation and tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Rudxain @AnonymousWP @Frigyes06 @lavafroth