Better assist rspec rails cop users when migrating to 3 #1931
Conversation
There was a problem hiding this comment.
You’re right, we forgot to change the obsoletion configuration before releasing v3.0.0, sorry about that.
A couple of things:
- We should probably use the
extractedkey instead,
extracted ofremoved, e.g.extracted: RSpec/Rails/*: rubocop-rspec_rails
- I don’t see that RuboCop tests the
extractedfeature with nested namespaces, so we should at least to manual testing to ensure it actually works. - In v3.0.0 we also extracted rubocop-factory_bot and rubocop-capybara, so we should fix the config for those cops as well.
Let me know if this is too much scope creep; I’m happy to fix it myself as well 😄
Nice, I was not aware of this feature, I just reused what was already in the config file 👍
I'll have a look
Makes sense. I took those from #1830, but it would obviously make sense to include the others.
I'll give it a shot, then hand it over if it grows too much in complexity. 😄 |
|
I've changed it as suggested. The extra layer of nesting seems to be working fine.
Will this warning trigger for all the affected cops? And is it okay that it's only a warning? |
@ydah what you think about this suggestion? |
There was a problem hiding this comment.
Wow, this is incredible! 🤩
I feel ashamed to have missed this, as it was indeed present in the rubocop's own obsoletion config.
LGTM, and thank you, @jeppester !
|
I can't push to your |
I would like to know how the output would look like if I add that. |
|
Perhaps the following changes should be implemented in rubocop-capybara, rubocop-factory_bot and rubocop-rspec_rails. # example of rubocop-capybara
extracted:
RSpec/Capybara/*: ~ |
|
I think that The output for the cases when the extracted gems aren’t bundled and are bundled, but extracted cops are still referenced by their old names are in this comment. I’d be most interested in edge cases, like:
In any case, I don’t see those as blockers, as this change makes the upgrade smoother anyway. |
Done. Let me know if there's more I need to do. |
|
@jeppester Could you squash two commits? |
Previously you would first get: ``` Error: The `RSpec/Rails/InferredSpecType` cop has been moved to `RSpecRails/InferredSpecType`. (obsolete configuration found in .rubocop.yml, please update it) ``` Then after renaming the cop, you would get: ``` Error: unrecognized cop or department RSpecRails/InferredSpecType found in .rubocop.yml Did you mean `RSpec/RepeatedDescription`? ``` Which would be a dead end.
Done |
Related issue: #1928
Previously you would first get:
Then after renaming the cop, you would get:
Which would be a dead end.
This is my attempt to lead users in a better direction.
I'm unsure about how to test this. Are there tests for obsoletion texts?
I'm also unsure if I need to check the documentation checkboxes for this?
Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).