RSpec/FilePath split is disabled by default

[splattael]

The changelog reads that RSpec/FilePath is "enabled by default" but it seems it isn't anymore.

Currently, users are forced to replace RSpec/FilePath with their alternatives when upgrading a minor release from 2.23.0 to 2.24.0. Is this expected or do we consider this a breaking change? 🤔

Refs https://gitlab.com/gitlab-org/ruby/gems/gitlab-styles/-/merge_requests/195

/cc @ydah @bquorning

Originally posted by @splattael in #1698 (comment)

[splattael]

From #1698 (comment):

@bquorning Sep 12, 2023

[...] I am not sure what is the preferred course of action – should we add the old cop back?

Good question! I was hoping we could turn the violation into a warning but unfortunately it's supported yet.

I see 2 options:

1. Fix the changelog and highlight that this is a breaking change
2. Enable FilePath by default and undo the split (in the config/obsoletion.yml) and highlight the tension of the cops in the changelog.

Later (or in the meanwhile):

• Allow split rules to emit warnings (via severity: warning in "cop rule", see above) in RuboCop repository
• Re-add the split configuration with severity: warning
• Make it a violation (instead of warning) in the next major release

Depending on what we consider a "breaking change" both options are possible 😅

WDYT?

[ydah]

As mentioned in the following PR

• Fix an error when using inherit_gem: rubocop-rspec: config/default.yml #1710

How about proceeding as follows?

1. Remove the configuration from config/obsoletion.yml once, since split would result in an error. (Next patch version release)
2. Release a version that warns if RSpec/FilePath is used (Next and subsequent releases)
3. Remove RSpec/FilePath in the next major version update

Later (or in the meanwhile):
Allow split rules to emit warnings (via severity: warning in "cop rule", see above) in RuboCop repository
Re-add the split configuration with severity: warning
Make it a violation (instead of warning) in the next major release

I think it looks good. Perhaps it would be better to implement it before step2.

[splattael]

@ydah 👋

How about proceeding as follows?

This sounds great!

I think it looks good. Perhaps it would be better to implement it before step2.

Yes, we could! However, we'd need to wait for the RuboCop version to be released and tweak the dependency here 🤷

For now, I only see either sticking with rubocop-rspec 2.23.2 or accept the breaking change and remove RSpec/FilePath from .rubocop.yml per project.

[pirj]

Sounds like a plan!

Cop rules currently can only be failures, not warnings

How did it work for our extractions?

[splattael]

Question for the maintainers: Since 2.24.0 has some potential for user confusion, I wonder if, after 2.24.1 was released with #1718 merged, we are considering yanking this release? 🤔

[pirj]

yanking this release

Quite unlikely. I see this as a regular bug fix.

We have had a dependency issue situation recently and planned to yank 2.18.0, but I think we didn’t and nobody complained.

[pirj]

Well, probably nobody complained because we haven’t released major versions yet.

[mockdeep]

I'm having trouble with this. If I disable the RSpec/FilePath rule, it complains that the rule has been deprecated, but if I remove references to it, it reports a violation. This is with EnabledByDefault: true.

[pirj]

Hey @mockdeep ! Interesting 🤔. Does it work the same way for you with the fix from #1718?

[mockdeep]

@pirj yeah, that fixes the issue for me. I need to handle both the FilePath and SpecFilePathFormat cops, but it doesn't cause an error preventing me from moving forward.

[bquorning]

v2.24.1 has been released, which I hope fixes the problem.

[edsu]

I'm confused, is the plan to still deprecate RSpec/FilePath?

[bquorning]

Yes, the RSpec/FilePath cop will be removed in v3.x (issue #1723). Since that would be a breaking change, the cop is still enabled for now. We did disable it in v2.24.0 and then re-enable it in v2.24.1, which I imagine may cause some confusion. (At least it did for me 😅)

If you upgraded to v2.24.0 and started have the new cops RSpec/SpecFilePathFormat and RSpec/SpecFilePathSuffix explicitly enabled, you probably want to explicit disable RSpec/FilePath after upgrading to v2.24.1.