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

Rename deprecated IgnoredPatterns config to AllowedPatterns #377

Merged
merged 1 commit into from May 2, 2022

Conversation

sambostock
Copy link
Contributor

@sambostock sambostock commented Apr 22, 2022

Also bumps Rubocop version.

Closes #376
Caused by rubocop/rubocop#10555
Closes #380

We could also switch to the !ruby/regexp syntax for the config, instead of a String, which would remove the need for all the escaping and clarify what the pattern is doing, but I can do that in a different PR.

@sambostock
Copy link
Contributor Author

I think this charge will require bumping the minimum Rubocop version (unless we can configure dynamically based on Rubocop version), and update the test that checks the full config.

@sambostock
Copy link
Contributor Author

🏓 @rafaelfranca & @volmer

Copy link
Contributor

@adrianna-chang-shopify adrianna-chang-shopify left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this Sam! I was going to look into this to fix the warnings when bumping Rubocop on Core CI, so it was nice to see you already got to it 😆

I was expecting to have to bump the rubocop version requirement as well -- I'm not aware of a way to handle different configs based on different versions.

@sambostock
Copy link
Contributor Author

#380 takes an alternative approach by using ERB to decide between configs by checking the Rubocop version, which allows us to handle the deprecation without forcing consumers to bump their Rubocop version.

However, I'm wondering if that matters at all? 🤔 Are we fine with requiring a version bump?

Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

I think a dependency version bump makes sense.

`IgnoredPatterns` has been renamed to `AllowedPatterns` and deprecated.

Switching to the new config requires the new version of Rubocop, and switching
to the new version requires using the new config because we forbid deprecations
in tests. Therefore, these changes have to happen together.
@sambostock sambostock requested a review from a team as a code owner May 2, 2022 19:18
@gjtorikian
Copy link

Could a version bump and release happen? 😅

@adrianna-chang-shopify
Copy link
Contributor

I can handle a new release 👍

@gjtorikian
Copy link

Thank you!

@shopify-shipit shopify-shipit bot temporarily deployed to rubygems May 13, 2022 19:59 Inactive
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.

Layout/LineLength obsolete parameter IgnoredPatterns
4 participants