-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add configuration for semicolon_block
lints
#10656
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
I think I will rewrite this later. I created this while half-asleep and so it's a bit more of a hacky solution than I'd like 🙂 It works currently but could be cleaner. |
Hey! Code written in the half-asleep state can be quite genius. At least there are quite a few PRs I created in that state which weren't too bad ^^. I planned to do a review of this on Wednesday, is that fine with you, or should I wait on the planned rewrite :) |
Wednesday sounds good, I think I can get it done by then. Thanks! e: Disregard this 😅 I think I may just refactor instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation overall looks good to me. I left two comments, where I would like your feedback on.
Also, because I didn't say that earlier: Welcome to Clippy 👋 📎
semicolon_outside_block_if_singleline
lintsemicolon_block
lints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally looks good to me, some nits and a few questions to make sure, that I understand everything correctly :)
Looks good to me, thank you for the update and addressing all my review comments. I hope you had fun working on Clippy :D @bors r+ |
Add configuration for `semicolon_block` lints Does exactly what it says on the tin, suggests moving a block's final semicolon inside if it's multiline and outside if it's singleline. I don't really like how this is implemented so I'm not too sure if this is ready yet. Alas, it might be ok. --- fixes #10654 changelog: Enhancement: [`semicolon_inside_block`]: Added `semicolon-inside-block-ignore-singleline` as a new config value. [#10656](#10656) changelog: Enhancement: [`semicolon_outside_block`]: Added `semicolon-outside-block-ignore-multiline` as a new config value. [#10656](#10656) <!-- changelog_checked -->
💔 Test failed - checks-action_test |
whoops, forgot to run |
Yes, that's what the CI is for. Thank you! @bors r+ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Does exactly what it says on the tin, suggests moving a block's final semicolon inside if it's multiline and outside if it's singleline.
I don't really like how this is implemented so I'm not too sure if this is ready yet. Alas, it might be ok.
fixes #10654
changelog: Enhancement: [
semicolon_inside_block
]: Addedsemicolon-inside-block-ignore-singleline
as a new config value.#10656
changelog: Enhancement: [
semicolon_outside_block
]: Addedsemicolon-outside-block-ignore-multiline
as a new config value.#10656