Skip to content
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

Deprecate IgnoredPatterns option in favor of the AllowedPatterns options #1336

Merged
merged 1 commit into from Sep 1, 2022

Conversation

ydah
Copy link
Member

@ydah ydah commented Jul 26, 2022

Follow up rubocop/rubocop#10555

This PR obsoletes the IgnoredPatterns option and replaces it with the AllowedPatterns option.


Before submitting the PR make sure the following are checked:

  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Updated documentation.
  • Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
  • The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

If you have created a new cop:

  • [-] Added the new cop to config/default.yml.
  • [-] The cop is configured as Enabled: pending in config/default.yml.
  • [-] The cop is configured as Enabled: true in .rubocop.yml.
  • [-] The cop documents examples of good and bad code.
  • [-] The tests assert both that bad code is reported and that good code is not reported.
  • [-] Set VersionAdded in default/config.yml to the next minor version.

If you have modified an existing cop's configuration options:

  • Set VersionChanged in config/default.yml to the next major version.

@ydah ydah force-pushed the ignored_patterns_to_allowed_patterns branch from 4db3706 to c230622 Compare July 26, 2022 01:03
@ydah
Copy link
Member Author

ydah commented Jul 26, 2022

rubocop/rubocop#10831 is required to resolve the build errors.

Copy link
Member

@Darhazer Darhazer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubocop/rubocop#10831 is required to resolve the build errors.

Does it also mean that we should bump the minimum required version? The AllowedPatterns is available since 1.28 and we already depend on 1.31, but if deprecation is broken, perhaps we should wait for the next release and also add dependency on it?

@@ -778,9 +778,10 @@ RSpec/VariableName:
SupportedStyles:
- snake_case
- camelCase
AllowedPatterns: []
IgnoredPatterns: []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove the obsolete configuration from the default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obsolete parameters are still available.
It seems to me that it would be better to keep both available for backward compatibility until the complete removal of the corresponding options at https://github.com/rubocop/rubocop, which will come someday.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime, is there a good way of suppressing this on every run?

Warning: obsolete parameter `IgnoredPatterns` (for `RSpec/VariableName`) found in vendor/bundle/ruby/3.1.0/gems/rubocop-rspec-2.13.2/config/default.yml
`IgnoredPatterns` has been renamed to `AllowedPatterns`.
Warning: obsolete parameter `IgnoredPatterns` (for `RSpec/VariableName`) found in .rubocop.yml
`IgnoredPatterns` has been renamed to `AllowedPatterns`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide .rubocop.yml?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning: obsolete parameter `IgnoredPatterns` (for `RSpec/VariableName`) found in .rubocop.yml
`IgnoredPatterns` has been renamed to `AllowedPatterns`.

This warning can be suppressed by modifying .rubocop.yml as follows.

before

RSpec/VariableName:
  IgnoredPatterns: [...]

after

RSpec/VariableName:
  AllowedPatterns: [...] 

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning: obsolete parameter `IgnoredPatterns` (for `RSpec/VariableName`) found in vendor/bundle/ruby/3.1.0/gems/rubocop-rspec-2.13.2/config/default.yml
`IgnoredPatterns` has been renamed to `AllowedPatterns`.

This is a guess, but I think the following settings may be included in the .rubocop.yml file.

inherit_from: vendor/bundle/ruby/3.1.0/gems/rubocop-rspec-2.13.2/config/default.yml

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I get the warnings with our .rubocop.yml like:

inherit_gem:
  rubocop-rspec:
    - config/default.yml

require:
  - rubocop-rspec

AllCops:
  TargetRubyVersion: 3.1
  NewCops: enable

I still get the warnings if I append this to the file -

RSpec/VariableName:
  AllowedPatterns: [] 

However, looking into it - I'm not so sure we need the inherit_gem bit. Seems like if I remove it we still get the rspec defaults, but don't get the deprecation warnings.

Sorry for the noise!

@ydah
Copy link
Member Author

ydah commented Jul 26, 2022

Does it also mean that we should bump the minimum required version? The AllowedPatterns is available since 1.28 and we already depend on 1.31, but if deprecation is broken, perhaps we should wait for the next release and also add dependency on it?

Yes. That is the recognition.
rubocop/rubocop#10831 has not been released yet, so it looks like we will have to wait for the next release.

@pirj
Copy link
Member

pirj commented Jul 26, 2022

rubocop/rubocop#10831 is required to resolve the build errors.

Does it also mean that we should bump the minimum required version?

But it's not released yet. This is why CI / spec Ruby 2.7 (edge) (rubocop master) passes, but other builds don't.

Let's wait for the fix to be released and bump the dependency.

@ydah ydah force-pushed the ignored_patterns_to_allowed_patterns branch from c230622 to bb9d2d9 Compare August 4, 2022 10:11
@ydah ydah requested a review from Darhazer August 4, 2022 10:13
@ydah
Copy link
Member Author

ydah commented Aug 4, 2022

Now we can incorporate this change:

@Darhazer
Copy link
Member

Darhazer commented Aug 4, 2022

we should also bump the minimum required rubocop version

@ydah ydah force-pushed the ignored_patterns_to_allowed_patterns branch 2 times, most recently from c06a85e to 8bd2838 Compare August 29, 2022 06:09
@ydah ydah changed the title Deprecate IgnoredPatterns option Deprecate IgnoredPatterns option in favor of the AllowedPatterns options Aug 29, 2022
@ydah
Copy link
Member Author

ydah commented Aug 29, 2022

@Darhazer Sorry for the delay in responding. I updated this PR.

@ydah ydah force-pushed the ignored_patterns_to_allowed_patterns branch from 8bd2838 to b658493 Compare August 30, 2022 10:51
@pirj pirj requested a review from bquorning August 30, 2022 13:03
…options

Follow up rubocop/rubocop#10555

This PR obsoletes the `IgnoredPatterns` option and
replaces it with the `AllowedPatterns` option.
@ydah ydah force-pushed the ignored_patterns_to_allowed_patterns branch from b658493 to 7befba0 Compare September 1, 2022 00:31
@bquorning
Copy link
Collaborator

@pirj / @Darhazer I’d like extra eyes on this PR. 🙏🏼

@Darhazer Darhazer merged commit 6b183b7 into rubocop:master Sep 1, 2022
@ydah ydah deleted the ignored_patterns_to_allowed_patterns branch September 1, 2022 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants