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

Make empty-line-after an early clippy lint #136657

Merged
merged 5 commits into from
Feb 8, 2025
Merged

Make empty-line-after an early clippy lint #136657

merged 5 commits into from
Feb 8, 2025

Conversation

jdonszelmann
Copy link
Contributor

r? @y21

95% a refiling of rust-lang/rust-clippy#13658 but for correctness it needed 2 extra methods in rust_lint which made it much easier to apply on rust-lang/rust than rust-lang/rust-clippy.

Commits have been thoroughly reviewed on rust-lang/clippy already. The last two review comments there (about using Option and popping for assoc items have been applied here.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 6, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@jdonszelmann jdonszelmann mentioned this pull request Feb 6, 2025
Closed
@Centri3
Copy link
Member

Centri3 commented Feb 6, 2025

it needed 2 extra methods in rust_lint which made it much easier to apply on rust-lang/rust than rust-lang/rust-clippy.

In the past, I've mostly seen such additions added in isolation to rustc with the rest kept in clippy. No objections from me, though?

@jdonszelmann
Copy link
Contributor Author

so in this case it was the other way around, 3 months ago we started with some changes in clippy so they'd be done in time for a PR on rust-lang/rust. In the end things ended up going differently and that PR was still open and now also needed some changes in rust-lang/rust so that'd have meant a sync both ways before the change landed. This'd shortcut that :)

@bors
Copy link
Contributor

bors commented Feb 7, 2025

☔ The latest upstream changes (presumably #136658) made this pull request unmergeable. Please resolve the merge conflicts.

flip1995
flip1995 approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM

@GuillaumeGomez
Copy link
Member

Nice, thanks to both of you!

y21
y21 approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

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

Looks also good to me, thank you. Sorry for taking longer with the review and blocking your HIR attribute refactor. This technically touches compiler code though and I don't know if I'm allowed to r+ those PRs even if just adding two lint context methods?

@y21
Copy link
Member

y21 commented Feb 7, 2025

So, taking the safe route, for the small compiler change that adds the two lint context methods that this lint needs: r? compiler

r=me and flip1995 if it looks good.

@rustbot rustbot assigned cjgillot and unassigned y21 Feb 7, 2025
WaffleLapkin
WaffleLapkin approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The compiler changes look good to me:3

@WaffleLapkin
Copy link
Member

r? WaffleLapkin

@bors r=y21,flip1995,WaffleLapkin

@bors
Copy link
Contributor

bors commented Feb 7, 2025

📌 Commit cd52a95 has been approved by y21,flip1995,WaffleLapkin

It is now in the queue for this repository.

@rustbot rustbot assigned WaffleLapkin and unassigned cjgillot Feb 7, 2025
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 8, 2025
@matthiaskrgr
Rollup merge of rust-lang#136657 - jdonszelmann:empty-line-after, r=y…
e4b7c45 
…21,flip1995,WaffleLapkin

Make empty-line-after an early clippy lint

r? `@y21`

95% a refiling of rust-lang/rust-clippy#13658 but for correctness it needed 2 extra methods in `rust_lint` which made it much easier to apply on `rust-lang/rust` than `rust-lang/rust-clippy`.

Commits have been thoroughly reviewed on `rust-lang/clippy already`. The last two review comments there (about using `Option` and popping for assoc items have been applied here.
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 8, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
@bors
Auto merge of rust-lang#136734 - matthiaskrgr:rollup-e6v3wwe, r=matth…
ddea0bb 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#131282 (std: detect stack overflows in TLS destructors on UNIX)
 - rust-lang#135696 (std: move `io` module out of `pal`, get rid of `sys_common::io`)
 - rust-lang#136099 (Optimize `Rc::<str>::default()` implementation)
 - rust-lang#136200 (Generate correct terminate block under Wasm EH)
 - rust-lang#136626 (create `initial_rustdoc` field in `Build`)
 - rust-lang#136657 (Make empty-line-after an early clippy lint)
 - rust-lang#136679 (ci: Use largedisk for loongarch)
 - rust-lang#136715 (Subtree sync for rustc_codegen_cranelift)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 8, 2025
@matthiaskrgr
Rollup merge of rust-lang#136657 - jdonszelmann:empty-line-after, r=y…
c8e0875 
…21,flip1995,WaffleLapkin

Make empty-line-after an early clippy lint

r? ``@y21``

95% a refiling of rust-lang/rust-clippy#13658 but for correctness it needed 2 extra methods in `rust_lint` which made it much easier to apply on `rust-lang/rust` than `rust-lang/rust-clippy`.

Commits have been thoroughly reviewed on `rust-lang/clippy already`. The last two review comments there (about using `Option` and popping for assoc items have been applied here.
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 8, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
@bors
Auto merge of rust-lang#136740 - matthiaskrgr:rollup-cy5405x, r=matth…
2e44d4e 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#135696 (std: move `io` module out of `pal`, get rid of `sys_common::io`)
 - rust-lang#136099 (Optimize `Rc::<str>::default()` implementation)
 - rust-lang#136200 (Generate correct terminate block under Wasm EH)
 - rust-lang#136626 (create `initial_rustdoc` field in `Build`)
 - rust-lang#136657 (Make empty-line-after an early clippy lint)
 - rust-lang#136679 (ci: Use largedisk for loongarch)
 - rust-lang#136715 (Subtree sync for rustc_codegen_cranelift)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 8, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
@bors
Auto merge of rust-lang#136747 - matthiaskrgr:rollup-qfiiv4n, r=matth…
43ca9d1 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#135696 (std: move `io` module out of `pal`, get rid of `sys_common::io`)
 - rust-lang#136099 (Optimize `Rc::<str>::default()` implementation)
 - rust-lang#136200 (Generate correct terminate block under Wasm EH)
 - rust-lang#136626 (create `initial_rustdoc` field in `Build`)
 - rust-lang#136657 (Make empty-line-after an early clippy lint)
 - rust-lang#136679 (ci: Use largedisk for loongarch)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dbcd74e into rust-lang:master Feb 8, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 8, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2025
@rust-timer
Unrolled build for rust-lang#136657
6cc6767 
Rollup merge of rust-lang#136657 - jdonszelmann:empty-line-after, r=y21,flip1995,WaffleLapkin

Make empty-line-after an early clippy lint

r? ```@y21```

95% a refiling of rust-lang/rust-clippy#13658 but for correctness it needed 2 extra methods in `rust_lint` which made it much easier to apply on `rust-lang/rust` than `rust-lang/rust-clippy`.

Commits have been thoroughly reviewed on `rust-lang/clippy already`. The last two review comments there (about using `Option` and popping for assoc items have been applied here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 approved these changes

@y21 y21 y21 approved these changes

@WaffleLapkin WaffleLapkin WaffleLapkin approved these changes

Assignees

@WaffleLapkin WaffleLapkin

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
9 participants
@jdonszelmann @rustbot @Centri3 @bors @GuillaumeGomez @y21 @WaffleLapkin @flip1995 @cjgillot