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.

Sign up for GitHub

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

Jump to bottom

Add an uber-department #1019

Merged
merged 2 commits into from Oct 22, 2020
Merged

Add an uber-department #1019

merged 2 commits into from Oct 22, 2020

Conversation

pirj
Copy link
Member

@pirj pirj commented Aug 22, 2020 •
edited by bquorning

To avoid name and department clash issues, RuboCop decided to grant each extension its own department.

More info rubocop/rubocop#8490

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.
  • CHECK ONCE AGAIN, e.g. Rails/PersistenceCalledOutsideExample
  • [-] 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).

@pirj pirj self-assigned this Aug 22, 2020
@pirj pirj added this to the 2.0 milestone Aug 22, 2020
@pirj pirj mentioned this pull request Aug 22, 2020
Merged
8 tasks
@dmytro-savochkin
Copy link

@pirj
Actually I think there was some confusion. Sorry for that. This PR rubocop/rubocop#8490 only changes departments for "deep" cops (that have 5 or more nesting level). For rubocop-rspec they are inside these directories:
https://github.com/rubocop-hq/rubocop-rspec/tree/master/lib/rubocop/cop/rspec/capybara
https://github.com/rubocop-hq/rubocop-rspec/tree/master/lib/rubocop/cop/rspec/factory_bot
https://github.com/rubocop-hq/rubocop-rspec/tree/master/lib/rubocop/cop/rspec/rails
I don't think it should affect any other cops from this gem.

@pirj
Copy link
Member Author

pirj commented Aug 23, 2020

@dmytro-savochkin Ah, I've missed that. Thanks a lot for the heads-up! Going to fix this shortly.

@pirj pirj mentioned this pull request Sep 17, 2020
Closed
10 tasks
@bquorning bquorning mentioned this pull request Oct 21, 2020
Closed
@bquorning
Copy link
Collaborator

@pirj I force-pushed to this branch, but the build is still failing. I’ll have a look at it later today.

@bquorning
Copy link
Collaborator

the build is still failing. I’ll have a look at it later today.

I think I fixed it :-)

@pirj
Copy link
Member Author

pirj commented Oct 21, 2020

Thanks! LGTM.

@bquorning bquorning changed the base branch from master to release-2.0 October 21, 2020 20:49
@bquorning bquorning force-pushed the uber-department branch 2 times, most recently from dccde97 to 7d20a20 Compare October 21, 2020 21:02
@bquorning
Copy link
Collaborator

  • Squashed into 2 commits.
  • Added Changelog entries.
  • Changed PR target branch.
  • Specs are passing. 🎉

I think we’re ready to merge.

@pirj @bquorning
Change department name of "nested" cops
ccf11f7 
To avoid name and department clash issues, RuboCop decided to grant each
extension its own department.
For those cops that already have the department matching the extension
name, no changes are needed.

More info rubocop/rubocop#8490

The changed cop names are:

* `Capybara/CurrentPathExpectation` -> `RSpec/Capybara/CurrentPathExpectation`
* `Capybara/FeatureMethods` -> `RSpec/Capybara/FeatureMethods`
* `Capybara/VisibilityMatcher` -> `RSpec/Capybara/VisibilityMatcher`
* `FactoryBot/AttributeDefinedStatically` -> `RSpec/FactoryBot/AttributeDefinedStatically`
* `FactoryBot/CreateList` -> `RSpec/FactoryBot/CreateList`
* `FactoryBot/FactoryClassName` -> `RSpec/FactoryBot/FactoryClassName`
* `Rails/HttpStatus` -> `RSpec/Rails/HttpStatus`
jesperronn
jesperronn approved these changes Oct 22, 2020
View reviewed changes
Copy link

@jesperronn jesperronn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is my first visit to rubocop-rspec code, this looks fairly safe to change (LGTM). I also think it is a good call in the changelog to call it "potentially breaking" although it is unlikely to be clashing with other rubocop extensions.

If I understand correctly, if you have configured these cops in your own .rubocop.yml/.rubocop_todo.yml, then you MUST also rename it there.

I would reckon it would help users to call this a breaking change. Good call 👍 .

@pirj
Copy link
Member Author

pirj commented Oct 22, 2020

@jesperronn We have a comprehensive document describing all the breaking changes in the 2.0 release and the migration procedure, you may check #1013 if you're interested.

@bquorning bquorning merged commit 0f09adb into release-2.0 Oct 22, 2020
@bquorning bquorning deleted the uber-department branch October 22, 2020 14:31
@bquorning bquorning mentioned this pull request Oct 22, 2020
Merged
4 tasks
@pirj pirj mentioned this pull request Nov 6, 2020
Closed
@pirj pirj mentioned this pull request Nov 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bquorning bquorning bquorning left review comments

@jesperronn jesperronn jesperronn approved these changes

@Darhazer Darhazer Awaiting requested review from Darhazer

Assignees

@pirj pirj

Labels
None yet
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pirj @dmytro-savochkin @bquorning @jesperronn