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

AutoCorrect: contextual needs rubocop v1.61 #1901

Merged
merged 2 commits into from
Jun 11, 2024
Merged

AutoCorrect: contextual needs rubocop v1.61 #1901

merged 2 commits into from
Jun 11, 2024

Conversation

ydah
Copy link
Member

@ydah ydah commented Jun 3, 2024

https://github.com/rubocop/rubocop/releases/tag/v1.61.0


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).

@ydah ydah requested a review from a team as a code owner June 3, 2024 18:05
@pirj
Copy link
Member

pirj commented Jun 3, 2024

The is is a big jump
Can we make it together with the 3.0 release, or in 3.x?

How did you catch this?
I see that rubocop/rubocop-rspec_rails@ff9bf2a is ❌, but I don’t see why.
Do we have this build job that checks with the lowest supported rubocop version?

@bquorning
Copy link
Collaborator

This makes me a bit hesitant to backport our contextual autocorrection to 2-x-stable.

@bquorning
Copy link
Collaborator

Do we have this build job that checks with the lowest supported rubocop version?

That would be https://github.com/rubocop/rubocop-rspec/actions/runs/9355023880/job/25749240673, using gem 'rubocop', '= 1.40' on e49f221.

@bquorning
Copy link
Collaborator

I see that rubocop/rubocop-rspec_rails@ff9bf2a is ❌, but I don’t see why.

The failing CI is because rubocop/rubocop-rspec_rails#30 is not (and cannot be) merged yet.

@bquorning
Copy link
Collaborator

I’m trying to read rubocop/rubocop#12657 and I think if people are using an older version of RuboCop than v1.61, autocorrect will just keep working as it always has. But if they’re using v1.61+ then they get the new LSP feature too.

In conclusion, I don’t think we need this PR. People who want to use LSP should use RuboCop v1.61+ anyway. But on the other hand, the target branch is master which is targeted at our v3.0, and we can decide to not backport to 2-x-stable, right?

@ydah ydah closed this Jun 4, 2024
@ydah ydah deleted the bump1_61 branch June 4, 2024 06:58
@ydah
Copy link
Member Author

ydah commented Jun 4, 2024

@bquorning Thanks for catching that. You're right, it is indeed. I'm going to close this PR since there is no need to necessarily update version it.

@bquorning
Copy link
Collaborator

Yeah, maybe we should have merged this before v3.0.0.
Reopening the PR as maybe we want to do it as a bugfix in v3.0.1?

@bquorning bquorning reopened this Jun 11, 2024
@ydah
Copy link
Member Author

ydah commented Jun 11, 2024

Let's do that. @pirj WDYT?

ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request Jun 11, 2024
ydah added a commit to rubocop/rubocop-rspec_rails that referenced this pull request Jun 11, 2024
ydah added a commit to rubocop/rubocop-rspec_rails that referenced this pull request Jun 11, 2024
ydah added a commit to rubocop/rubocop-factory_bot that referenced this pull request Jun 11, 2024
@bquorning
Copy link
Collaborator

I added one commit, so that merging this PR will release v3.0.1.

I feel like I have been a little too eager to merge and release lately, so I’ll wait for @pirj to review before merging.

@pirj
Copy link
Member

pirj commented Jun 11, 2024

Let’s merge and release as 3.0.1, i see no big harm here.

The only problem would be people doing bundle update —conservative rubocop-rspec, which would resolve to 3.0.0 as 3.0.1 would depend on newer rubocop and won’t match.

I’d be most interested in why our CI “oldest rubocop” job didn’t catch this.

@ydah
Copy link
Member Author

ydah commented Jun 11, 2024

In the following sections, if AutoCorrect was set to a value other than boolean, the corresponding error message was output, so it was necessary to update to v1.61. So, let's change it and release it.

https://github.com/rubocop/rubocop/blob/edb56c927d4d1112a9c02990da31767e68d2121c/lib/rubocop/config_validator.rb#L251-L268

@bquorning bquorning merged commit 955e485 into master Jun 11, 2024
24 checks passed
@bquorning bquorning deleted the bump1_61 branch June 11, 2024 15:34
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.

3 participants