-
-
Notifications
You must be signed in to change notification settings - Fork 248
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
Change terminology to ForbiddenMethods
and AllowedMethods
#265
Conversation
dfbcac6
to
927a117
Compare
This new names looks good, but this change need a mechanism that does not cause an error due to the breaking change. |
@koic how should I do that? I don't see a mechanism to extend https://github.com/rubocop-hq/rubocop/blob/6b9510ddab76d3052ed12e1a5e299ad407180ade/lib/rubocop/config_obsoletion.rb#L5 |
I don't have an answer yet. I was considering this rename, but didn't change it due to the breaking changes yet. |
I guess the simplest way would be to support the old and the new config name for a while, but advertise only the new name and issue some warning if the old one is bound. |
I've added a commit to warn about obsolete configs, is this an acceptable approach? |
Yeah, the deprecation logic may be tweaked later, anyway I think this approach is acceptable 👍 After merging #266, let's proceed with this :-) |
#266 has been merged. Can you rebase with the latest master branch? |
Use terminology which is more descriptive and is consistent with the direction of some upstream libraries: rubocop/rubocop#6467 rails/rails#33681
@koic rebased. |
Thanks! |
```bash bundle exec rake Running RuboCop... Warning: Rails/SkipsModelValidations does not support Blacklist parameter. Supported parameters are: - Enabled - ForbiddenMethods - AllowedMethods ``` See rubocop/rubocop-rails#265 and alphagov#112
Use terminology which is more descriptive and is consistent with the direction of some upstream libraries:
rubocop/rubocop#6467
rails/rails#33681