RuboCop: Stage IV #171
Open
RuboCop: Stage IV #171
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
tagliala
force-pushed
the
chore/rubocop-stage-iv
branch
from
33b654b
to
9d62078
Compare
tagliala
commented
Oct 10, 2024
| @@ -66,7 +65,7 @@ def add_custom_transformation(options) | |||
| raise InlineSvg::Configuration::Invalid.new("#{options.fetch(:transform)} should implement the .create_with_value and #transform methods") | |||
| end | |||
|
|
|||
| @custom_transformations.merge!(Hash[*[options.fetch(:attribute, :no_attribute), options]]) | |||
| @custom_transformations.merge!(options.fetch(:attribute, :no_attribute) => options) | |||
There was a problem hiding this comment.
Manual fix 1: looks safe to me
tagliala
commented
Oct 10, 2024
| @@ -65,7 +64,7 @@ def self.without_empty_values(params) | |||
| def self.all_default_values | |||
| custom_transformations | |||
| .values | |||
| .select { |opt| opt[:default_value] != nil } | |||
| .reject { |opt| opt[:default_value].nil? } | |||
There was a problem hiding this comment.
Manual fix 2: looks safe to me
tagliala
commented
Oct 10, 2024
| end | ||
| rescue NameError | ||
| raise InlineSvg::Configuration::Invalid.new("asset_file should implement the #named method") | ||
| end |
There was a problem hiding this comment.
suggestion: review with "hide whitespaces"
tagliala
force-pushed
the
chore/rubocop-stage-iv
branch
3 times, most recently
from
85766aa
to
609a448
Compare
|
@jamesmartin rebased and ready for review Most of the changes are related to non-production code There are two changes on production code with a manual fix, they have a comment and I'm quite positive they are safe |
jamesmartin
reviewed
Oct 14, 2024
Comment on lines
27
to
31
| transforms.inject({}) do |output, (name, definition)| | ||
| transforms.each_with_object({}) do |(name, definition), output| | ||
| priority = definition.fetch(:priority, built_in_transformations.size) | ||
|
|
||
| output[name] = definition.merge({ priority: magnify(priority) }) | ||
| output |
There was a problem hiding this comment.
Not sure about this one. I know some people don't like that it's necessary to remember to return the memo object from the block but I've never loved the explicit mutability of the object in each_with_object. 🤔
There was a problem hiding this comment.
I've disabled this cop with a reference to this comment and force pushed
- `Layout/DotPosition` - `Lint/AmbiguousOperator` - `Lint/LiteralInInterpolation` - `RSpec/BeEmpty` - `RSpec/ContextMethod` - `RSpec/HookArgument` - `RSpec/MatchArray` - `RSpec/NotToNot` - `Style/ExpandPathArguments` - `Style/HashSyntax` - `Style/NestedParenthesizedCalls` - `Style/RedundantBegin` - `Style/RedundantConstantBase` - `Style/RedundantReturn` - `Style/RegexpLiteral` Some manual fixes: - `Lint/RedundantSplatExpansion` and subsequent unsafe manual fix of `Style/HashConversion` - `Style/NonNilCheck` and subsequent switch unsafe manual fix of `Style/InverseMethods`
tagliala
force-pushed
the
chore/rubocop-stage-iv
branch
from
609a448
to
40c7a16
Compare
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.
Requires #170
I've tried to only fix objective or almost-objective offenses, if you don't like some of the fixes let me know.
There are very few changes to production code
This PR fixes:
Layout/DotPositionLint/AmbiguousOperatorLint/LiteralInInterpolationRSpec/BeEmptyRSpec/ContextMethodRSpec/HookArgumentRSpec/MatchArrayRSpec/NotToNotStyle/EachWithObjectStyle/ExpandPathArgumentsStyle/HashSyntaxStyle/NestedParenthesizedCallsStyle/RedundantBeginStyle/RedundantConstantBaseStyle/RedundantReturnStyle/RegexpLiteralSome manual fixes:
Lint/RedundantSplatExpansionand subsequent unsafemanual fix of
Style/HashConversionStyle/NonNilCheckand subsequent switch fromselect !niltoreject nil