rubocop / rubocop-rspec

Code style checking for RSpec files.
https://docs.rubocop.org/rubocop-rspec
MIT License
794 stars 272 forks source link

Extract RSpec Rails cops #1830

Closed ydah closed 3 months ago

ydah commented 4 months ago

Fix: https://github.com/rubocop/rubocop-rspec/issues/1734

TODO

Here's where to temporary extract it:.


Before submitting the PR make sure the following are checked:

ydah commented 4 months ago

@rubocop/rubocop-rspec We can now proceed to extraction 🎉 @bquorning Could you create https://github.com/rubocop/rubocop-rspec_rails?

pirj commented 4 months ago

Should we do rubocop-rspec_rails with underscore lije we do for rubocop-factory_bot? As a repo name and in the code especially: we’ll use RuboCop::RSpecRails as the namespace, right?

ydah commented 4 months ago

Should we do rubocop-rspec_rails with underscore lije we do for rubocop-factory_bot? As a repo name and in the code especially: we’ll use RuboCop::RSpecRails as the namespace, right?

I was wondering whether it should be separated by hyphen or underscore. In factory_bot, the name of the original gem is separated by underscore, but rspec-rails is separated by hyphen. Does the name rspec_rails sound strange? If it doesn't sound strange, I will change the name of the repository. @rubocop/rubocop-rspec I'd like to hear your opinion.

bquorning commented 4 months ago

Based on https://guides.rubygems.org/name-your-gem/ and e.g. https://github.com/ydah/rubocop-rspec-rails/blob/207fc3483e75605b12276b90e5bb3a7ebcdc223f/lib/rubocop/cop/rspec_rails/avoid_setup_hook.rb defining RuboCop::Cop::RSpecRails and not RuboCop::Cop::RSpec::Rails, I think the gem should be named rubocop-rspec_rails.

ydah commented 4 months ago

Thank you. we use rubocop-rspec_rails instead of rubocop-rspec-rails.

ydah commented 4 months ago

I updated this and extracted PR.

bquorning commented 4 months ago

I created https://github.com/rubocop/rubocop-rspec_rails with @rubocop/rubocop-core given write-access. Ping me if anyone needs admin access.

ydah commented 4 months ago

@rubocop/rubocop-rspec If the modifications on the rubocop-rspec_rais side are correct, I will apply them to the https://github.com/rubocop/rubocop-rspec_rails . Please review https://github.com/ydah/rubocop-rspec_rails/pull/1 .

pirj commented 4 months ago

I can’t spot any rails-specific modules in our code at a glance, so all good.

ydah commented 3 months ago

I overlooked something serious. Gems cannot depend on each other.

❯ bundle install
Your bundle requires gems that depend on each other, creating an infinite loop. Please remove either gem 'rubocop-rspec' or gem 'rubocop-rspec_rails' and try again.
pirj commented 3 months ago

I guess we’ll have to remove the dependency from rubocop-rspec_rails and make a note there that it implicitly depends on rubocop-rspec. In 3.0, we’ll remove the dependency on rubocop-rspec_rails from rubocop-rspec, and will restore the dependency declaration in rubocop-rspec_rails. This might be a mild surprise to those who would want just rubocop-rspec_rails, but I suppose it’s a very rare case.

ydah commented 3 months ago

@pirj Should rubocop-rspec_rails be a patch upgrade or a minor upgrade?

pirj commented 3 months ago

I’d yank the 2.28.0 and released a bugfix patch in 2.28.1. @bquorning what do you think?

bquorning commented 3 months ago

I’d yank the 2.28.0 and released a bugfix patch in 2.28.1.

Yes that sounds like a good idea. And I assume nobody is using the gem yet, so the yank won’t affect users.

ydah commented 3 months ago

I update this PR. And All Green âś…