-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
rubocop: Clean up Style/BlockDelimiters
excludes and autofix offenses
#14920
Conversation
Review period will end on 2023-03-08 at 23:00:45 UTC. |
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.
Thanks for the PR @issyl0! Let's change specs rather than the non-spec code here, if we want consistency.
@@ -152,10 +152,10 @@ def find_launchctl_with_wildcard(search) | |||
regex = Regexp.escape(search).gsub("\\*", ".*") | |||
system_command!("/bin/launchctl", args: ["list"]) | |||
.stdout.lines.drop(1) # skip stdout column headers | |||
.map do |line| | |||
.map { |line| |
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.
Sorry, really don't agree with this change. Would rather we enforced specs to use this format than change non-spec code for this.
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.
So you want EnforcedStyle: line_count_based
(the default) everywhere (so for multi-line blocks they're always do ... end
?
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.
@issyl0 Yes I think so.
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.
Done!
e3fc20d
to
b753677
Compare
Review period ended. |
b753677
to
1be614e
Compare
- The defaults of using "do ... end" for multi-line blocks everywhere is good, better than switching everything to braces everywhere.
1be614e
to
3a83b54
Compare
Nice, thanks @issyl0! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Exclude
withrubocop
#14685, maybe?!EnforcedStyle: braces_for_chaining
config makes the most sense. It preserves a lot of the RSpec syntax we're used to inexpect {}
blocks that are chained to other things - quite a lot of them - and uses normal block syntax for non-chained methods (as the name suggests - I probably didn't need to write that down).