-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
Declare autocorrect as unsafe for RSpec/ReceiveMessages
#1687
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
We can also follow up after this PR to make autocorrect as safe as possible 👍🏻 |
098a426
to
233faf2
Compare
Historically I was always against marking cops as unsafe if they were buggy. |
I agree with you. However, I believe that the case mentioned in the next comment may be difficult to design. |
Ah sorry @pirj, I only just saw your comment now. I had just merged #1688 and was about to push this change as a bugfix release (but I’ll wait, pending this discussion).
My idea was exactly to temporarily mark the cop as unsafe, and then have a potential subsequent PR make the cop safe again. Can you elaborate on why you prefer not having temporarily unsafe cops? |
There’s nothing more permanent than temporary 😅 |
Ah yes, that is true. However in this case we have released a bug (claiming the autocorrect is safe) and the fastest way to fix it is to stop claiming the autocorrect is safe. Issue #1677 is still open, so we (someone) can pick it up and implement safe autocorrect. Would it be ok if I proceed with the v2.23.1 release? |
Please go ahead with the release. |
As suggested in #1677.
I am unsure if I should change
VersionChanged
for the cop too.Before submitting the PR make sure the following are checked:
master
(if not - rebase it).CHANGELOG.md
if 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).If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"
inconfig/default.yml
.