-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply inherit_mode to Include and Exclude of AllCops #16
Apply inherit_mode to Include and Exclude of AllCops #16
Conversation
AllCops: | ||
TargetRubyVersion: TODO | ||
Include: | ||
- '**/Rakefile' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[FYI]
Because **/Rakefile
includes in the default configs.
- '**/config.ru' | ||
Exclude: | ||
- 'Gemfile' | ||
- 'bin/*' | ||
- 'config/**/*' | ||
- 'db/**/*' | ||
- 'deploy/**/*' | ||
- 'vendor/**/*' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[FYI]
Because vendor/**/*
includes in the default configs.
Because those settings exist in the default setting of rubocop.yml
9ed7051
to
83f9786
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
README.md
Outdated
AllCops: | ||
TargetRubyVersion: TODO | ||
Include: | ||
- '**/Rakefile' | ||
- '**/config.ru' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nits]
'**/config.ru' also seems listed in the default config. so it can be deleted, i think.
ref. https://github.com/rubocop-hq/rubocop/blob/c50e7473f8627dd276cb7656bcd1953cb74bb243/config/default.yml#L31
Currently, the
rubocop
is working in pixta's repository with using configs and examples ofpixta-rubocop
. But it doesn't report problems when a code has problems.Because from
rubocop
v0.56.0 it changed the behavior ofAllCops/Include
andAllCops/Exclude
to the overriding way.https://github.com/rubocop-hq/rubocop/blob/master/relnotes/v0.56.0.md#new-features
You can see this issue and pull request.
rubocop/rubocop#4247
rubocop/rubocop#5882
Oh my God... I will check that with using configs and examples of
pixta-rubocop
in a new rails v4.2.x project. I used this.rubocop.yml
(Refer to README ofpixta-rubocop
).This one is the result. The target file is
Rakefile
andconfig.ru
only. It can't use default configs ofrubocop
.We should fix that problem. Normally
pixta-rubocop
uses default configs ofrubocop
. But we customized some configs to PIXTA. So we also want to apply that mind toAllCops/Include
andAllCops/Exclude
. If that we can useinherit_mode
.inherit_mode
has two optionsmerge
andoverride
.https://docs.rubocop.org/en/stable/configuration/#merging-arrays-using-inherit_mode
In our case, we can specify
inherit_mode/merge
toAllCops/Include
andAllCops/Exclude
.I also check that with using
inherit_mode/merge
.This one is the result. it works well 💯
This PR changes README and examples in the
pixta-rubocop
. After this PR is LGTM, we need to apply that configs to pixta's repository.