fix clippy issues #1905
fix clippy issues #1905
Conversation
sync-team/src/github/tests/mod.rs
Outdated
| @@ -19,7 +19,11 @@ fn team_create() { | |||
| let user = model.create_user("mark"); | |||
| let user2 = model.create_user("jan"); | |||
| let gh = model.gh_model(); | |||
| model.create_team(TeamData::new("admins").gh_team(DEFAULT_ORG, "admins-gh", &[user, user2])); | |||
| model.create_team(TeamData::new_builder("admins").gh_team( | |||
There was a problem hiding this comment.
I had to rename new because that method wasn't returning Self. So I called the method new_builder.
An alternative is calling it just builder
There was a problem hiding this comment.
build? It's annoying though, I would just silence the lint on this method.
There was a problem hiding this comment.
I also agree to silence that lint.
There was a problem hiding this comment.
Personally, I'd stick with the convention here and rename the method to something like builder. We're probably all using code completion, so there's imo no cost in adding a few characters to the method name. 😅
But not a blocking concern, happy to go with whatever the majority thinks...
There was a problem hiding this comment.
I went for #[allow(clippy::new_ret_no_self)] since it's the less destructive option. We can always remove it in another PR
marcoieni
force-pushed
the
fix-clippy-issues
branch
3 times, most recently
from
bae4776 to
32be30c
Compare
marcoieni
requested review from
Mark-Simulacrum,
pietroalbini and
jdno
as code owners
Dry-run check results |
| - name: Run clippy | ||
| run: cargo clippy --workspace --all-targets --all-features -- -D warnings |
There was a problem hiding this comment.
this will imply that contributions to the codebase must now pass clippy, and no warnings allowed.
I see that a clippy lint is enforcing two rules:
#![allow(clippy::enum_variant_names)]#[allow(clippy::type_complexity)]
There was a problem hiding this comment.
Also one question (for my curiosity): I think I don't see a specific clippy lint config file. What happens if clippy "stabilizes" and adds a new default?
There was a problem hiding this comment.
this will imply that contributions to the codebase must now pass clippy
yes. I find clippy is a good teacher for new contributors. They learn about idiomatic rust 👍
and no warnings allowed
I think that all clippy lints are treated as warnings, so if you don't add -D warnings then this step will pass 😅
What happens if clippy "stabilizes" and adds a new default?
clippy adds new defaults on every rust release more or less. We pin the rust version in CI, so whenever we bump the rust version we need to fix new clippy lints, too (if any)
|
Is the diff mostly stylistic changes? How do they improve the quality of the code? |
the code is more readable imo e.g. |
marcoieni
force-pushed
the
fix-clippy-issues
branch
from
32be30c to
7aec8c7
Compare
Let's run clippy in CI to improve the quality of the codebase