Redo the Rubocop config #564
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
Many cops were explicitly enabled. We don't necessarily need to do that since the defaults of RuboCop are always included. So we just specify where we deviate from RuboCops default values.
This has as consequence that whenever we upgrade RuboCop, we should run checks on the whole codebase to see if new cops somehow don't work with our code. We can then discuss if we maybe want to explicitly disable a new cop.
This comment was marked as off-topic.
This comment was marked as off-topic.
This was referenced Nov 15, 2023
|
@fosterfarrell9 Apart from the comment you gave, do you think we can merge? |
fosterfarrell9
approved these changes
Nov 25, 2023
Splines
added a commit
that referenced
this pull request
Jan 6, 2024
* Shorten RuboCop config Many cops were explicitly enabled. We don't necessarily need to do that since the defaults of RuboCop are always included. So we just specify where we deviate from RuboCops default values. * Add comment regarding "NewCops" * Add "NewCops: enable" This has as consequence that whenever we upgrade RuboCop, we should run checks on the whole codebase to see if new cops somehow don't work with our code. We can then discuss if we maybe want to explicitly disable a new cop. * Update RuboCop related packages in Gemfile * Add WordArray & EmptyMethod enforced styles * Sort RuboCop keys alphabetically & improve file comment
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.
Related:
See the new
.rubocop.ymlfrom this PR.Automatically Generated Configuration. And some useful commands.
Some stats using the new config at commit e768bfc:
This newly created todo file "only" contains 37 offenses that couldn't be autocorrected (although for some we get the comment they can be using unsafe autocorrection, but we've done that in the step beforehand?! -> don't know why this is the case). This file can be found here, maybe we want to disable some of the cops from there already...
(Note: of course one also has to watch our carefully when running rubocop with unsafe autocorrects as this could possibly modify the code in a way that is semantic and not just syntactic. A strategy for a future PR could therefore consist of running the safe autocorrects first and commit, then do a separate commit for just unsafe autocorrects. This will ease the review process. Finally, we have the rubocop todo file that we can either directly try to work off, or do it from time to time, e.g. in another PR).