Skip to content

[Merged by Bors] - fix new clippy lints #5160

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

Closed

Conversation

jakobhellermann
Copy link
Contributor

Objective

  • Nightly clippy lints should be fixed before they get stable and break CI

Solution

for line in io::stdin().lines() {
    ...
}

@jakobhellermann jakobhellermann added the C-Code-Quality A section of code that is hard to understand or change label Jul 1, 2022
@mockersf
Copy link
Member

mockersf commented Jul 1, 2022

it would be better to wait for the new lint to be fixed rather than add an ignore for an unstable lint


struct Input(String);

fn my_runner(mut app: App) {
println!("Type stuff into the console");
for line in io::stdin().lock().lines() {
#[allow(clippy::significant_drop_in_scrutinee)] // https://github.com/rust-lang/rust-clippy/issues/8963
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to avoid ignoring this for as long as we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PR still contains the change

- use std::io::BufRead;

-     for line in io::stdin().lock().lines() { 
+     for line in io::stdin().lines() { 

which was stabilized in 1.62 and has an equivalent implementation https://doc.rust-lang.org/stable/std/io/struct.Stdin.html#method.lines.

I can revert the change if it shouldn't be in this clippy fix PR.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with this change being included.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

In agreement with Francois above. But the rest of the changes look good to me.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 1, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 1, 2022
# Objective

- Nightly clippy lints should be fixed before they get stable and break CI
  
## Solution

- fix new clippy lints
- ignore `significant_drop_in_scrutinee` since it isn't relevant in our loop rust-lang/rust-clippy#8987
```rust
for line in io::stdin().lines() {
    ...
}
```

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
@bors bors bot changed the title fix new clippy lints [Merged by Bors] - fix new clippy lints Jul 1, 2022
@bors bors bot closed this Jul 1, 2022
@jakobhellermann jakobhellermann deleted the fix-clippy-lints branch July 4, 2022 14:28
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

- Nightly clippy lints should be fixed before they get stable and break CI
  
## Solution

- fix new clippy lints
- ignore `significant_drop_in_scrutinee` since it isn't relevant in our loop rust-lang/rust-clippy#8987
```rust
for line in io::stdin().lines() {
    ...
}
```

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

- Nightly clippy lints should be fixed before they get stable and break CI
  
## Solution

- fix new clippy lints
- ignore `significant_drop_in_scrutinee` since it isn't relevant in our loop rust-lang/rust-clippy#8987
```rust
for line in io::stdin().lines() {
    ...
}
```

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Nightly clippy lints should be fixed before they get stable and break CI
  
## Solution

- fix new clippy lints
- ignore `significant_drop_in_scrutinee` since it isn't relevant in our loop rust-lang/rust-clippy#8987
```rust
for line in io::stdin().lines() {
    ...
}
```

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants