Layout Cops #9203
Merged
Layout Cops #9203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit enables the Rubocop Layout/SpaceAfterNot cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceAfterSemicolon cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceAroundBlockParameters cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceAroundEqualsInParameterDefault cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceAroundKeyword cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceAroundMethodCallOperator cop and addresses all offenses.
The Puppet::Pops::Validation::Diagnostic#== method triggers multiple Rubocop Layout offenses. Despite that, it's unclear if there is another way to format the method to improve readability. This commit disables all Rubocop Layout cops on this method.
This commit enables the Rubocop Layout/SpaceAroundOperators cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceBeforeBlockBraces cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceBeforeBrackets cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceBeforeComma cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceBeforeFirstArg cop.
This commit enables the Rubocop Layout/SpaceBeforeSemicolon cop and addresses all offenses.
Prior to this commit, the lexer used arrays of sequential characters
(0-9, a-z, A-Z) by explicitly naming each character in the array (e.g.
"['0', '1', '2']").
This commit simplifies those arrays to use ranges (e.g. "('0'..'9')")
instead.
This commit enables the Rubocop Layout/SpaceInsideArrayLiteralBrackets cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideBlockBraces cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideHashLiteralBraces cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideParens cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsidePercentLiteralDelimiters cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideRangeLiteral cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideReferenceBrackets cop and addresses all offenses.
This commit enables the Rubocop Layout/SpaceInsideStringInterpolation cop and addresses all offenses.
This commit enables the Rubocop Layout/TrailingEmptyLines cop and addresses all offenses.
This commit enables the Rubocop Layout/TrailingWhitespace cop and addresses all offenses.
mhashizume
force-pushed
the
PUP-11768/main/layout-and-cops-6
branch
from
e46b5ba
to
6cb3b8c
Compare
|
Note that I've punted on one cop here, Layout/LineLength. I generally think it would be good to have some kind of warning or check for line length since there are some especially egregious offenders (the longest I found was almost 600 characters long), but it would be pretty involved to implement that cop. |
joshcooper
reviewed
Jan 11, 2024
|
|
||
| [ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9'].each do |c| | ||
| ('0'..'9').each do |c| |
There was a problem hiding this comment.
More thinking out loud, are there perf implications to this? We generally have to be careful about changes to the lexer, even simple things like referencing an instance variable vs a local variable. It might be worth creating a sample test and running it in benchmark-ips with JRuby 9.4 and see if there's any appreciable difference?
There was a problem hiding this comment.
Running the benchmark here:
puppet/spec/unit/pops/benchmark_spec.rb
Lines 83 to 87 in 97f7854
The performance seems comparable between this branch and main, both with MRI and JRuby:
| Environment | Results |
|---|---|
| MRI 3.2.2, main | Lexer2: 0.245682 0.016103 0.261785 ( 0.264757) |
| MRI 3.2.2, PUP-11768/main/layout-and-cops-6 | Lexer2: 0.242654 0.014749 0.257403 ( 0.261339) |
| JRuby 9.4.3.0,main | Lexer2: 2.540000 0.040000 2.580000 ( 0.799291) |
| JRuby 9.4.3.0, PUP-11768/main/layout-and-cops-6 | Lexer2: 2.380000 0.030000 2.410000 ( 0.766099) |
joshcooper
reviewed
Jan 11, 2024
joshcooper
reviewed
Jan 11, 2024
joshcooper
reviewed
Jan 11, 2024
| @@ -305,7 +305,7 @@ def self.needs_ensure_retrieved | |||
|
|
|||
| validate do | |||
| if @parameters[:logonpassword] && @parameters[:logonaccount].nil? | |||
| raise Puppet::Error.new(_"The 'logonaccount' parameter is mandatory when setting 'logonpassword'.") | |||
| raise Puppet::Error.new(_("The 'logonaccount' parameter is mandatory when setting 'logonpassword'.")) | |||
joshcooper
approved these changes
Jan 11, 2024
joshcooper
approved these changes
Jan 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the final PR in this series for enabling and resolving Rubocop Layout cops.