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

build: move needless_borrow lint allows to be global #722

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

jeckersb
Copy link
Contributor

@jeckersb jeckersb commented Jul 24, 2024

A new instance of this snuck in under xtask, this will make sure it's
covered everywhere going forward.

Signed-off-by: John Eckersberg jeckersb@redhat.com

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I'm OK with this but can you explain why you're adding this?

Isn't this covered by the usage of global lints in Cargo.toml now?

Is this prep for another PR?

@jeckersb
Copy link
Contributor Author

I'm OK with this but can you explain why you're adding this?

Isn't this covered by the usage of global lints in Cargo.toml now?

Is this prep for another PR?

Hm no it looks like the global lints aren't covering it. I just noticed that clippy started complaining after https://github.com/containers/bootc/pull/703/files#diff-1b2f83403e0dae2d6172ad76c50e66039ec03402104f90900a29203790d21051R144

@jeckersb
Copy link
Contributor Author

Looks like I can fix it globally by just adding it to the root Cargo.toml so I'll change this to do that instead.

@cgwalters
Copy link
Collaborator

I'm not getting any errors from make validate on current git main though. Oh...is this rustc version drift? My toolchain refresh cycle is a bit slow, was on 1.77. Just updated to 1.79, but I still don't see any clippy warnings on current git main.

A new instance of this snuck in under xtask, this will make sure it's
covered everywhere going forward.

Signed-off-by: John Eckersberg <jeckersb@redhat.com>
@jeckersb
Copy link
Contributor Author

I'm not getting any errors from make validate on current git main though. Oh...is this rustc version drift? My toolchain refresh cycle is a bit slow, was on 1.77. Just updated to 1.79, but I still don't see any clippy warnings on current git main.

With stable/1.79 I get:

$ cargo +stable clippy
   Compiling bootc-lib v0.1.13 (/var/home/jeckersb/git/bootc/lib)
    Checking tests-integration v0.1.0 (/var/home/jeckersb/git/bootc/tests-integration)
    Checking xtask v0.1.0 (/var/home/jeckersb/git/bootc/xtask)
warning: the borrowed expression implements the required traits
   --> xtask/src/xtask.rs:144:28
    |
144 |     std::fs::write(target, &schema)?;
    |                            ^^^^^^^ help: change this to: `schema`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
    = note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default

warning: `xtask` (bin "xtask") generated 1 warning (run `cargo clippy --fix --bin "xtask"` to apply 1 suggestion)
    Checking bootc v0.1.9 (/var/home/jeckersb/git/bootc/cli)
    Finished `dev` profile [optimized + debuginfo] target(s) in 3.42s

@jeckersb jeckersb changed the title xtask: allow clippy needless borrows build: move needless_borrow lint allows to be global Jul 24, 2024
@jeckersb jeckersb enabled auto-merge July 24, 2024 16:24
@jeckersb jeckersb merged commit 9d149a1 into containers:main Jul 24, 2024
8 of 28 checks passed
@jeckersb jeckersb deleted the xtask_needless_borrows branch July 24, 2024 16:34
@cgwalters
Copy link
Collaborator

I'm a bit confused as I don't get that with the same rust version; but it's OK no need to dig in now...

@jeckersb
Copy link
Contributor Author

I'm a bit confused as I don't get that with the same rust version; but it's OK no need to dig in now...

Was/is your local main branch behind upstream? #703 only went in last night.

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