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

[Fix #9095] use merged_config instead of config for pending new cop check #9096

Merged
merged 1 commit into from Nov 30, 2020
Merged

[Fix #9095] use merged_config instead of config for pending new cop check #9096

merged 1 commit into from Nov 30, 2020

Conversation

ThomasKoppensteiner
Copy link
Contributor

Fix for issue #9095

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 25, 2020

Did you manage to figure out when the regression was introduced? Was this in 1.3?

Also - ideally you should add some spec illustrating the impact of the fix.

@ThomasKoppensteiner
Copy link
Contributor Author

@bbatsov the code was last changed within this commit, but before it also used config.
I think the issue is more related to the introduction of the NewCops option and how it disables the warning.

@bbatsov bbatsov requested a review from koic November 26, 2020 16:22
@ThomasKoppensteiner
Copy link
Contributor Author

For me it looks like the issue is related to the monkey patching which happens in rubocop-rspec or rt_rubocop_defaults. At least I couldn't reproduce it in otherwise.

@ThomasKoppensteiner
Copy link
Contributor Author

Btw. is this override still needed, when Rainbow.enabled = false is set in the spec_helper.rb?

@ThomasKoppensteiner
Copy link
Contributor Author

The following spec fails for me on master as well bundle exec rspec spec/rubocop/cop/style/redundant_regexp_escape_spec.rb:523.

@ThomasKoppensteiner
Copy link
Contributor Author

@bbatsov and @koic anything else I can do here?

@koic
Copy link
Member

koic commented Nov 27, 2020

The following spec fails for me on master as well bundle exec rspec spec/rubocop/cop/style/redundant_regexp_escape_spec.rb:523.

This failure has been resolved by #9102. Can you rebase with the latest master branch?

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 28, 2020

Seems to me something went wrong with the rebase, as a lot of commits on master appeared in the diff for this branch.

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 28, 2020

Also - don't forget to add a bug-fix entry in the changelog folder.

@ThomasKoppensteiner
Copy link
Contributor Author

Seems to me something went wrong with the rebase, as a lot of commits on master appeared in the diff for this branch.

Thank you for pointing this out. Did it again, now with command line only.

@ThomasKoppensteiner ThomasKoppensteiner changed the title Fix #9095 use merged_config instead of config for pending new cop check [Fix #9095] use merged_config instead of config for pending new cop check Nov 29, 2020
@bbatsov bbatsov merged commit b266e67 into rubocop:master Nov 30, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 30, 2020

Thanks!

@ThomasKoppensteiner ThomasKoppensteiner deleted the fix-pending-new-cop-check branch November 30, 2020 07:23
@ThomasKoppensteiner
Copy link
Contributor Author

@bbatsov do you already know, when this will be released? What is the typical release cycle of rubocop?

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 1, 2020

I plan to do the next release today or tomorrow. Typically I'm aiming for one small release each week/couple of weeks, but I don't pursue any particular release cadence.

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

3 participants