(PUP-11771) Rubocop Naming Cops #9183
Merged
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
The private method is_module_package? was only ever referenced by code that was removed in 2014 by 3f05274. Because this method is both private and not referenced anywhere else in the code base, this commit removes the is_module_package? method.
Heredoc delimiters do not bring much value and would involve a lot of work to change existing delimiters. This commit disables the Rubocop Naming/HeredocDelimiterNaming cop.
The Naming/MethodParameterName Rubocop cop is fairly arbitrary. This commit disables it.
Disable the Naming/ClassAndModuleCamelCase Rubocop cop on existing violations, as class names are public by default and updating them could be a breaking change. Enable this cop for new code to catch any future violations.
Disable the Naming/ConstantName Rubocop cop on existing violations, as those constants may be public and updating them could be a breaking change. Enable this cop for new code to catch any future violations.
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
2 times, most recently
from
89b793a
to
8ef4974
Compare
Because renaming memoized instance variables can be potentially dangerous, to address the Rubocop Naming/MemoizedInstanceVariableName cop, this commit excludes existing violations while enabling the cop for new code.
Pops implements the visitor pattern and that relies on the convention of methods being namedoperation_Class. This commit excludes puppet/functions, puppet/pops, and puppet/util/windows from the Rubocop Naming/MethodName cop. Additionally, this commit updates the rpm_compareEVR methods in both Puppet::Util::Package::Version::Rpm and Puppet::Util::RpmCompare to instead be called rpm_compare_evr to adhere to snake case.
This commit disables the Rubocop Naming/RescuedExceptionsVariableName cop, as the cost of updating all of the existing violations outweighs the benefits of those changes.
This commit enables the Rubocop Naming/VariableName cop while excluding files that had been previously excluded (some Windows code that intentionally follows Microsoft's Hungarian notation). New code will be subject to this cop.
This commit disables the Rubocop Naming/VariableNumber cop. This check affects too much of our existing codebase and the cost of correcting potential violations outweighs the benefits of enabling this cop.
This commit disables the Rubocop Naming/BinaryOperatorParameterName cop, as its violations are too far-reaching in our codebase to safely and easily fix.
This commit enables the Rubocop Naming/BlockParameterName cop.
This commit moves the Rubocop Naming/AccessorMethodName cop from the todo file to the .rubocop configuration file. This does not change any behavior but indicates that this behavior is known and intentional.
The is_constant_defined? method was added in 3d43d86 to account for behavior differences between Ruby 1.8 and 1.9. It was simplified in 84a9cb8 as part of removal Ruby 1.8 support and the method exists now as a fairly transparent wrapper of Ruby's built-in Module#const_defined? method. is_constant_defined? has been marked as private since 79889af in 2013. This method removes the is_constant_defined? method and substitutes it with Module#const_defined? instead.
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
2 times, most recently
from
550bd2e
to
60e143d
Compare
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
from
60e143d
to
5d4ea14
Compare
joshcooper
reviewed
Dec 14, 2023
joshcooper
reviewed
Dec 14, 2023
joshcooper
reviewed
Dec 14, 2023
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
from
5d4ea14
to
21bbb92
Compare
|
@joshcooper Thanks for the review, I've reverted the issues you've flagged |
|
Ah sorry, the Naming/PredicateName cop is confusing... I was hoping to keep the |
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
from
21bbb92
to
e7e59ba
Compare
|
@joshcooper Gotcha, I think I understand now. I've updated my commit with your suggestions. Let me know if I can make any other changes. |
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
from
e7e59ba
to
ac34d58
Compare
joshcooper
reviewed
Dec 15, 2023
This commit addresses the Rubocop Naming/PredicateName cop by either exempting public methods with predicate names or renaming private methods.
mhashizume
force-pushed
the
PUP-11771/main/rubocop-naming
branch
from
ac34d58
to
40d11ec
Compare
joshcooper
approved these changes
Dec 19, 2023
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 PR comprises commits to address Rubocop naming cops and associated issues.