Update the versions of all linters #153
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
santiagovidal
force-pushed
the
enhancement/update_linters
branch
2 times, most recently
from
7455548
to
81190ea
Compare
santiagovidal
requested review from
santib,
federicoaldunate,
diebarral,
Alxzu,
anthfb,
BelenRemedi,
dani24gl,
fedeagripa,
f19ps,
gabrielbursztein2,
gonzaM,
grilix,
GuilleLeopold,
jdrosales17 and
jpascual1994
GuilleLeopold
approved these changes
Feb 26, 2019
santib
approved these changes
Feb 26, 2019
fedeagripa
approved these changes
Feb 26, 2019
gabrielbursztein2
approved these changes
Feb 26, 2019
jpascual1994
approved these changes
Feb 26, 2019
jdrosales17
approved these changes
Feb 26, 2019
BelenRemedi
approved these changes
Feb 26, 2019
diebarral
approved these changes
Feb 26, 2019
grilix
reviewed
Feb 26, 2019
| max_allowed_nesting: 2 | ||
| ignore_iterators: [] | ||
| NilCheck: | ||
| enabled: false |
There was a problem hiding this comment.
Any chance we can enable this one? We see a lot of nil checks 🤔
There was a problem hiding this comment.
It makes sense in theory, but the rule is just too aggressive. For example, in ActAsApiRequest it was complaining about this line: request_content_type&.match?(/json/)
Basically it will complain if we use safe navigation or try.
There was a problem hiding this comment.
Okay, yeah.. I really don't like the try, but I think the &. is not that bad and we should allow it. Thanks for the clarification.
grilix
reviewed
Feb 26, 2019
.reek.yml
Outdated
| accept: [] | ||
| UncommunicativeModuleName: | ||
| enabled: true | ||
| exclude: ['V1'] |
There was a problem hiding this comment.
Can we have this one as a regex? Something like, for example /V\d/? Not sure on how it's going to resolve the conflict with the reject list, tho.
santiagovidal
force-pushed
the
enhancement/update_linters
branch
from
81190ea
to
9d8d476
Compare
grilix
approved these changes
Feb 26, 2019
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 updates the versions of the following linters:
rubocopreekrails_best_practicesbrakemanOf those updates, the major change is
reek, which we are moving from version 4 to 5. There are some important changes torubocopas well.Changes to
reek:4.7.2to5.3.1.reek.ymlChanges to
rubocop:0.49.1to0.65.0Layout/ClassStructure(disabled by default).Style/ReturnNil(disabled by default).Style/ExpandPathArguments(enabled by default).Railscops (disabled by default). Of all the Rails cops, the following cops are different from the default configuration:Rails/FilePathis disabledRails/SaveBangis enabled