Fix auto-correctable Rubocop violations (feedback addressed) #2474
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
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
It looks as if these were fixed by the previously applied fixes for the violations of the `Style/BarePercentLiterals` cop.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
All these fixes were generated using Rubocop's auto-correct option.
I chose to fix these violations manually, because I wasn't happy with the changes generated by Rubocop's auto-correct mechanism.
I chose to fix these violations manually, because I wasn't happy with the changes generated by Rubocop's auto-correct mechanism.
I chose to fix some of these violations manually, because I wasn't happy with the changes generated by Rubocop's auto-correct mechanism. I also chose to disable the cop for a couple of the violations, because I couldn't see a way to satisfy the cop without making the code much less readable. The code in question is pretty horrible and will hopefully go away when we get round to refactoring these flows. I feel that disabling the cops in these cases just serves to highlight the problem code and should encourage us to fix it!
I fixed these violations manually, because Rubocop's auto-correct mechanism didn't actually do anything.
I chose to fix these violations manually, because I wasn't happy with the changes generated by Rubocop's auto-correct mechanism.
I chose to fix these violations manually, because I wasn't happy with the changes generated by Rubocop's auto-correct mechanism.
All these fixes were generated using Rubocop's auto-correct option. I think the generated fixes can be improved upon, but I plan to do that in separate commits to make the manual changes more obvious.
This method is only ever invoked from within the `show` action of this controller and it is always invoked with a block. This change makes explicit use of the `block` argument to make the code more readable while continuing to avoid violating the `Lint/UnusedMethodArgument` cop.
Instances of `HolidayEntitlement` should respond to existing methods as well as the special formatting methods. Note that this has allowed me to remove the leading underscore on the `include_all` argument while still avoiding violating the `Lint/UnusedMethodArgument` cop.
I want to supply different numbers of arguments to each of the sub-classes of `SmartAnswer::Node` and this seems like the simplest way.
This removes the unused `_options` or `options` argument from the constructor of `SmartAnswer::Node` and its subclasses. This makes the arguments expected for each constructor more explicit.
I don't think this method is serving much purpose. If the `transition` method is called on an instance of `Outcome`, a `NoMethodError` exception will be raised which should give just as much (if not more) information than this `InvalidNode` exception.
This means we can avoid the underscore prefix in the `_blk` argument just to avoid the `Lint/UnusedMethodArgument` cop violation.
All these flows have been changed in the fixes for Rubocop violations. None of them have resulted in changes to the externally-observable behaviour and all the regression tests passed without any updates to their artefacts. Thus I'm happy to update these checksums.
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.
Description
Supersedes #2469.
Trello card: https://trello.com/c/BqZ8i1b6
This PR fixes all the violations of cops which are supposedly auto-correctable. There are still quite a few disabled cops, but hopefully this is a good step forward.
All the commits up to and including the one fixing the
Style/AndOrviolations were generated using only Rubocop's auto-correct functionality by runninggovuk-lint-ruby --auto-correct.The next batch of commits up to and including the one fixing the
Lint/UnusedBlockArgumentviolations were done manually. In the middle of this batch, the commit fixing theStyle/TrivialAccessorsviolation was done manually, because the auto-correct option didn't seem to do anything.The commit fixing the
Lint/UnusedMethodArgumentviolations was generated using only Rubocop's auto-correct functionality, but then the subsequent 7 commits attempt to make improvements to those auto-corrections.Finally I've updated all the affected regression checksums in a single commit - no regression test artefacts needed to be changed to make the regression tests pass.
External changes
None. This is just an internal refactoring.