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

Extract Rails Cops in a separate #5976

Closed
bbatsov opened this issue Jun 9, 2018 · 23 comments

Comments

@bbatsov
Copy link
Collaborator

commented Jun 9, 2018

Rails cops have to extracted into a standalone gem (named something like rubocop-rails or whatever).

The reason for this decision is simple. I want RuboCop to be focused just on Ruby and have no framework/library/runtime dependant pieces for the sake of being easier to focus on evolving it. Creating more extensions should also help us come up with a good official API for them.

Any volunteers to do this? I don't do much Rails anymore, so I'd rather not be involved in this project if possible. @koic I know you've been working a lot on the Rails cops, so perhaps you'd like to tackle the extraction?

@bbatsov bbatsov added this to the 1.0.0 milestone Jun 9, 2018

@Drenmi

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2018

Unfortunately someone hogged the rubocop-rails name on RubyGems. I think the gem hosts the configuration used by the Rails team.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 9, 2018

Yeah, I noticed this, but someone on RubyKaigi mentioned the guy would be willing to give the name up. I'm fine with naming the extension rubocop-ror or whatever.

@composerinteralia

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2018

I would be happy to do this, or to help @koic do it. I wrote the couple of custom cops in rails and had a lot of fun doing it. (@koic I'm guessing we take those custom cops out of rails once the next rubocop version gets released. I would be happy to make that PR and to update rubocop-rails
as well unless you were planning to do it.)

@Drenmi

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2018

@koic Maybe you can reach out to toshimaru about the name? His e-mail is registered in RubyGems.

@koic

This comment has been minimized.

Copy link
Member

commented Jun 9, 2018

@bbatsov Yes. I will work on this issue. I'm thinking about planning now.

The following are issues and solutions for realizing this plan. There are still some issues to consider 🤔
Please let me know if you have any opinion.

RuboCop Rails Gem's name

As discussed above in this Issue, rubocop-rails gem already exists.
https://rubygems.org/gems/rubocop-rails

@Drenmi I'm asking @toshimaru whether he can transfer the gem name of rubocop-rails to the RuboCop team.

Even if toshimaru accepts the transfer gem name ownership, it seems necessary to announce to the current rubocop-rails gems (toshimaru/rubocop-rails) users, so we will need to talk to toshimaru about it.

@toshimaru Sorry to ask this of you, would you transfer rubocop-rails gem name to the RuboCop team, is it possible? I think that for the Rails department managed by RuboCop HQ, the name RuboCop Rails (rubocop-rails) is user-friendly as a convention.

Transition from RuboCop Core to RuboCop Rails

I'm thinking about the incompatibility impact on Rails department users.

Rails department users suddenly become surprised if Rails department cops are absent from RuboCop Core. I think that it is better to prepare the transition period.

So I'm considering the following transition procedure.

First, gemify RuboCop Rails and cut out.

  • Copy the Rails department from RuboCop Core repo to RuboCop Rails repo
  • Release RuboCop Rails gem (before this there is document description and some work 😉)

Prepare the following warning for RuboCop Core.

For example, the following warning is considered.

DEPRECATION WARNING: Rails department is separated and will be removed in
RuboCop 0.y.z. Please use rubocop-rails gem.

Finally, remove the Rails department from RuboCop Core. We will need to consider when removting it.

However, this procedure temporarily duplicates Rails department to RuboCop Core repo and RuboCop Rails repo. Therefore maintenance may be twice troublesome, perhaps it may be better to temporarily s.add_runtime_dependency on RuboCop Rails from RuboCop Core (validation is required).

@composerinteralia I'd like to think about a happy procedure for the Rails community as well during the transition.

About the existing issues of the Rails department

I'm thinking about the method and cost of migrating the issue to RuboCop Rails. Perhaps I will leave the existing issues to the Rails department in the RuboCop Core repo as it is, and it seems good to treat it with RuboCop Rails repo only in future issues.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 10, 2018

However, this procedure temporarily duplicates Rails department to RuboCop Core repo and RuboCop Rails repo. Therefore maintenance may be twice troublesome, perhaps it may be better to temporarily s.add_runtime_dependency on RuboCop Rails from RuboCop Core (validation is required).

I think that we can probably just cut out the new gem and show some instructions about adding the new gem when people try to enable the Rails cops. Seems this will be painless and won't really created breakages for the users (they'll just end up running fewer check temporary). I don't like much the idea of having the cops at some point in both RuboCop and RuboCop Rails, because this opens up the question of do we fix bugs in both gems or just in the new one.

We should leave some stubs for the Rails cops (or just some generic fallback logic) - e.g. each cop from the Rails department that's present in .rubocop.yml just generates a message that this cop does nothing and people need to install the new gem.

I agree that we should try to make this transition smooth, but I also think we shouldn't go overboard with it, as that's going to be one of the final big changes before 1.0. I think most users would forgive us some slight inconvenience now for the sake of long term stability.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 10, 2018

I've created a repo https://github.com/rubocop-hq/rubocop-rails so you can start hacking there. We'll renaming it later if we fails to obtain the rubocop-rails name on RubyGems.

@koic

This comment has been minimized.

Copy link
Member

commented Jun 10, 2018

We should leave some stubs for the Rails cops (or just some generic fallback logic) - e.g. each cop from the Rails department that's present in .rubocop.yml just generates a message that this cop does nothing and people need to install the new gem.

I see. I didn't think of it that way, and it looks good! 🌟
First we need gemifiy, so I will work on it. Thank you for letting me know.

@toshimaru

This comment has been minimized.

Copy link

commented Jun 11, 2018

@koic @bbatsov

would you transfer rubocop-rails gem name to the RuboCop team, is it possible?

Yes, It’s okay to give up the name if rubocop team really want it!

If I gave the name, who’d be the the author of the gem??

@Drenmi

This comment has been minimized.

Copy link
Collaborator

commented Jun 12, 2018

@toshimaru 🙇

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 12, 2018

@toshimaru I guess for now you can make me and @koic the owners. My handle on RubyGems is bozhidar, not sure about @koic's.

@koic

This comment has been minimized.

Copy link
Member

commented Jun 12, 2018

@toshimaru I appreciate your acceptance very much. My RubyGems account is koic_ito (koic.ito@gmail.com). Thank you!

@koic

This comment has been minimized.

Copy link
Member

commented Oct 1, 2018

The current situation. Currently, RuboCop Rails and Performance repositoies are extracting from Core using cp command. So, the git history has not been extracted now. This is inconvenient for future maintenance.

I'm investigating to extract some git history with git filter-branch or something.

After finishing the investigating, I'd like to proceed with application to the RuboCop Rails and Performance repositories. (And I may propose to RuboCop Performance first).

I appreciate your patience as it might take some time.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 1, 2018

@koic Thanks for the update! I was wondering recently how are we doing on this front.

Hopefully you'll manage to find a way to preserve the history, but if you don't - we can live without it. It's not ideal, but it's not the end of the world either.

@koic koic referenced this issue Oct 5, 2018

Closed

Disable cop Rails/ActiveRecordAliases for rails < 4 #6357

8 of 8 tasks complete
@koic

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

FYI: I cherry-picked the git histories.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2018

@koic Fantastic!

So what else remains before we can cut a release of rubocop-rails?

@koic

This comment has been minimized.

Copy link
Member

commented Oct 27, 2018

@bbatsov Thanks!

I think this issue is aimed at extracting Rails department code from RuboCop Core in the future.
For that reason there is work remain to extract TargetRailsVersion from RuboCop Core to rubocop-rails. So I'm trying to extract TargetRailsVersion.

Also, I think there is some documentation about Rails department to be extracted. (Detailed documentation may be good even after release.)

@koic

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

@bbatsov I have a question.

Do we extract metadata (#5978) to rubocop-rails and rubocop-performance from RuboCop Core?
If we extract metadata, we need to consider release versioning of rubocop-rails and rubocop-performance.

What do you have any ideas about this?

Related issue and PR: rubocop-hq/rubocop-rspec#699 and #6419.

@bquorning

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

I am curious whether RuboCop-Rails and RuboCop-Performance would not be subject to the same namespace issues that RuboCop-RSpec has been seeing. RuboCop-RSpec has an RSpec/Rails/HttpStatus cop and an RSpec/FilePath cop that sometimes conflict with RuboCop’s Rails/HttpStatus and Rails/FilePath cops. See issues like rubocop-hq/rubocop-rspec#611, #5251, and rubocop-hq/rubocop-rspec#710.

RuboCop-RSpec seemingly still needs the RuboCop::RSpec::Inject hack, which adds to my suspicion that we are not yet entirely ready to split out the Rails and Performance cops.

@bbatsov

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 3, 2018

Do we extract metadata (#5978) to rubocop-rails and rubocop-performance from RuboCop Core?
If we extract metadata, we need to consider release versioning of rubocop-rails and rubocop-performance.

Yeah, we'll move the metadata to the gems. Each gem will have an independent release cycle, so ideally we'll have to come up with a reasonable stable extension API during the course of this extraction process. :-)

I am curious whether RuboCop-Rails and RuboCop-Performance would not be subject to the same namespace issues that RuboCop-RSpec has been seeing. RuboCop-RSpec has an RSpec/Rails/HttpStatus cop and an RSpec/FilePath cop that sometimes conflict with RuboCop’s Rails/HttpStatus and Rails/FilePath cops. See issues like rubocop-hq/rubocop-rspec#611, #5251, and rubocop-hq/rubocop-rspec#710.

We'll fix such issues while tackling the migration. None of them are particularly complex.

RuboCop-RSpec seemingly still needs the RuboCop::RSpec::Inject hack, which adds to my suspicion that we are not yet entirely ready to split out the Rails and Performance cops.

Well, it's widely known that RuboCop doesn't have a proper extension API right now, but the best way to devise the API (mostly a ways for extensions to registers themselves properly, without the need for custom hacks) is to actually write a few extensions that will help the API emerge and stabilize. :-)

rmm5t added a commit to rmm5t/rubocop that referenced this issue May 22, 2019

Merge branch 'master' into 7066-fix-layout-align-hash-mixed-hash-styles
* master:
  Add a Note for Including/Excluding files
  Switch to `receive` message expectation syntax
  Simplify spec for FormatterSet
  [rubocop-hq#5976] Warn for Rails Cops
@halfbyte

This comment has been minimized.

Copy link

commented May 23, 2019

Can someone please add a short note in the rubocop-rails README about the renaming/owner change?

I am inclined to say that the name takeover is a bad idea that should have never happened (from a platform stability standpoint) but I also know that it's way too late to chime in on that. You are probably aware that the yanking is also gonna break a ton of builds of the users of the old gem, so there's that as well.

In any case, not mentioning this anywhere in the README will leave a lot of people quite confused as to what exactly has happened, so it would be great if you could fix that.

koic added a commit to koic/rubocop-rails that referenced this issue May 24, 2019

Add a reference to `rubocop-rails_config` gem in README.md
This PR adds a reference to `rubocop-rails_config` gem in README.md

Fixes rubocop-hq#56.
Follow up rubocop-hq#57 (comment).
Follow up rubocop-hq/rubocop#5976 (comment).

koic added a commit to koic/rubocop-rails that referenced this issue May 24, 2019

[Fix rubocop-hq#56] Add a reference to `rubocop-rails_config` gem in …
…README.md

This PR adds a reference to `rubocop-rails_config` gem in README.md

Fixes rubocop-hq#56.
Follow up rubocop-hq#57 (comment).
Follow up rubocop-hq/rubocop#5976 (comment).

koic added a commit to koic/rubocop-rails that referenced this issue May 24, 2019

[Fix rubocop-hq#56] Add a reference to `rubocop-rails_config` gem in …
…README.md

This PR adds a reference to `rubocop-rails_config` gem in README.md

Fixes rubocop-hq#56.
Follow up rubocop-hq#57 (comment).
Follow up rubocop-hq/rubocop#5976 (comment).

koic added a commit to koic/rubocop that referenced this issue May 30, 2019

[rubocop-hq#5976] Warn for `rubocop -R/--rails` option
This PR warns for `rubocop -R/--rails` option.

Here is an example using `-R/--rails` option:

```console
% cat app/models/user.rb
class User < ActiveRecord::Base
end
```

```console
% rubocop -R
`-R/--rails` option and Rails cops will be removed from RuboCop
0.72. Use the `rubocop-rails` gem instead.
https://github.com/rubocop-hq/rubocop/blob/master/manual/migrate_rails_cops.md
Inspecting 2 files
.C

Offenses:

(snip)

app/models/user.rb:1:14: C: Rails/ApplicationRecord: Models should
subclass ApplicationRecord.
class User < ActiveRecord::Base
             ^^^^^^^^^^^^^^^^^^
```

This PR provides a way to dismiss the above warning.
Specify `require: rubocop-rails` in .rubocop.yml (or command option)
instead of the `-R/-rails` option as indicated in the above URL of warning.

And this means that it will be used the same way as RuboCop RSpec and
RuboCop Performance.

This is the migration path to remove Rails cops from the RuboCop core,
Rails cops will still be disabled by default for compatibility.

koic added a commit to koic/rubocop that referenced this issue May 30, 2019

[rubocop-hq#5976] Warn for `rubocop -R/--rails` option
This PR warns for `rubocop -R/--rails` option.

Here is an example using `-R/--rails` option:

```console
% cat app/models/user.rb
class User < ActiveRecord::Base
end
```

```console
% rubocop -R
`-R/--rails` option and Rails cops will be removed from RuboCop
0.72. Use the `rubocop-rails` gem instead.
https://github.com/rubocop-hq/rubocop/blob/master/manual/migrate_rails_cops.md
Inspecting 2 files
.C

Offenses:

(snip)

app/models/user.rb:1:14: C: Rails/ApplicationRecord: Models should
subclass ApplicationRecord.
class User < ActiveRecord::Base
             ^^^^^^^^^^^^^^^^^^
```

This PR provides a way to dismiss the above warning.
Specify `require: rubocop-rails` in .rubocop.yml (or command option)
instead of the `-R/-rails` option as indicated in the above URL of warning.

And this means that it will be used the same way as RuboCop RSpec and
RuboCop Performance.

This is a migration path to remove Rails cops from the RuboCop core,
Rails cops will still be disabled by default for compatibility.

bbatsov added a commit that referenced this issue May 30, 2019

[#5976] Warn for `rubocop -R/--rails` option
This PR warns for `rubocop -R/--rails` option.

Here is an example using `-R/--rails` option:

```console
% cat app/models/user.rb
class User < ActiveRecord::Base
end
```

```console
% rubocop -R
`-R/--rails` option and Rails cops will be removed from RuboCop
0.72. Use the `rubocop-rails` gem instead.
https://github.com/rubocop-hq/rubocop/blob/master/manual/migrate_rails_cops.md
Inspecting 2 files
.C

Offenses:

(snip)

app/models/user.rb:1:14: C: Rails/ApplicationRecord: Models should
subclass ApplicationRecord.
class User < ActiveRecord::Base
             ^^^^^^^^^^^^^^^^^^
```

This PR provides a way to dismiss the above warning.
Specify `require: rubocop-rails` in .rubocop.yml (or command option)
instead of the `-R/-rails` option as indicated in the above URL of warning.

And this means that it will be used the same way as RuboCop RSpec and
RuboCop Performance.

This is a migration path to remove Rails cops from the RuboCop core,
Rails cops will still be disabled by default for compatibility.

koic added a commit to koic/rubocop that referenced this issue May 31, 2019

[Fix rubocop-hq#5976] Remove Rails cops
Fixes rubocop-hq#5976.

This PR removes Rails department from RuboCop core.

- Remove Rails department cops
- Remove `-R/--rails` option from `rubocop` command

These were transferred to RuboCop Rails gem.
https://github.com/rubocop-hq/rubocop-rails

Some internal implementations of `TargetRailsVersion` remain. The final
goal in the near future is to extract `TargetRailsVersion`
implementation to RuboCop Rails from RuboCop core.
rubocop-hq/rubocop-rails#47

This PR aims removing Rails cops and `rubocop -R /-rails` option that
affect RuboCop 0.72.0 users.

It may be better to merge this PR when we decide the next release
version is RuboCop 0.72.

@koic koic referenced this issue May 31, 2019

Merged

[Fix #5976] Remove Rails cops #7095

7 of 8 tasks complete

koic added a commit to koic/rubocop that referenced this issue May 31, 2019

[Fix rubocop-hq#5976] Remove Rails cops
Fixes rubocop-hq#5976.

This PR removes Rails department from RuboCop core.

- Remove Rails department cops
- Remove `-R/--rails` option from `rubocop` command

These were transferred to RuboCop Rails gem.
https://github.com/rubocop-hq/rubocop-rails

Some internal implementations of `TargetRailsVersion` remain. The final
goal in the near future is to extract `TargetRailsVersion`
implementation to RuboCop Rails from RuboCop core.
rubocop-hq/rubocop-rails#47

This PR aims removing Rails cops and `rubocop -R /-rails` option that
affect RuboCop 0.72.0 users.

It may be better to merge this PR when we decide the next release
version is RuboCop 0.72.

koic added a commit to koic/rubocop that referenced this issue May 31, 2019

[Fix rubocop-hq#5976] Remove Rails cops
Fixes rubocop-hq#5976.

This PR removes Rails department from RuboCop core.

- Remove Rails department cops
- Remove `-R/--rails` option from `rubocop` command

These were transferred to RuboCop Rails gem.
https://github.com/rubocop-hq/rubocop-rails

Some internal implementations of `TargetRailsVersion` remain. The final
goal in the near future is to extract `TargetRailsVersion`
implementation to RuboCop Rails from RuboCop core.
rubocop-hq/rubocop-rails#47

This PR aims removing Rails cops and `rubocop -R /-rails` option that
affect RuboCop 0.72.0 users.

It may be better to merge this PR when we decide the next release
version is RuboCop 0.72.

@bbatsov bbatsov closed this in #7095 Jun 2, 2019

bbatsov added a commit that referenced this issue Jun 2, 2019

[Fix #5976] Remove Rails cops
Fixes #5976.

This PR removes Rails department from RuboCop core.

- Remove Rails department cops
- Remove `-R/--rails` option from `rubocop` command

These were transferred to RuboCop Rails gem.
https://github.com/rubocop-hq/rubocop-rails

Some internal implementations of `TargetRailsVersion` remain. The final
goal in the near future is to extract `TargetRailsVersion`
implementation to RuboCop Rails from RuboCop core.
rubocop-hq/rubocop-rails#47

This PR aims removing Rails cops and `rubocop -R /-rails` option that
affect RuboCop 0.72.0 users.

It may be better to merge this PR when we decide the next release
version is RuboCop 0.72.

vzamanillo added a commit to vzamanillo/linter-rubocop that referenced this issue Jun 14, 2019

Removed configuration to run Rubocop with extra Rails cops
According to rubocop-hq/rubocop#5976 Rails department was removed from Rubocop core and extracted to a plugin, anyway this option had no effect because was removed accidentaly on e9c0319

vzamanillo added a commit to vzamanillo/linter-rubocop that referenced this issue Jun 14, 2019

fix: removed configuration to run Rubocop with extra Rails cops
According to rubocop-hq/rubocop#5976 Rails department was removed from Rubocop core and extracted to a plugin, anyway this option had no effect because was removed accidentaly on e9c0319

app2641 added a commit to grooves/forkwell_cop that referenced this issue Jun 26, 2019

Mr0grog added a commit to edgi-govdata-archiving/web-monitoring-db that referenced this issue Jul 10, 2019

Add rubocop-rails gem
In the latest rubocop, all the Rails-related cops were extracted into a new rubocop-rails gem, so this adds that gem. For more, see: rubocop-hq/rubocop#5976

Mr0grog added a commit to edgi-govdata-archiving/web-monitoring-db that referenced this issue Jul 10, 2019

Add rubocop-rails gem
In the latest rubocop, all the Rails-related cops were extracted into a new rubocop-rails gem, so this adds that gem. For more, see: rubocop-hq/rubocop#5976

tonyta added a commit to tonyta/adopt-a-drain that referenced this issue Jul 26, 2019

Add rubocop-rails gem (Rails cops were extracted in Rubocop 0.72.0)
Rubocop was updated to v0.72.0 in 9efc6a0. That version extracted Rails
cops to a separate gem: rubocop rails.

  rubocop-hq/rubocop#5976

This commit reintroduces the missing Rails cops.

ifricker added a commit to q-centrix/hound that referenced this issue Aug 6, 2019

Remove all references to rails in rubocop config
Rubocop extracted all rails checks to separate gem
rubocop-hq/rubocop#5976

artur-beljajev added a commit to internetee/style-guide that referenced this issue Aug 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.