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
Add new RSpec/EmptyHooks
cop
#897
Conversation
236a885
to
1f2d97c
Compare
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.
Looks good, thank you!
A few things can be improved in the cop itself, and the spec might be made more human-readable.
Ping |
1f2d97c
to
aaebd28
Compare
@pirj Fixed. Please review. |
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.
Looks incredible, good job!
A few really minor notes concerning simplification.
4a2000d
to
3db8502
Compare
@pirj Changes done. |
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.
You can add autocorrect that removes the offending node
3db8502
to
f228ac3
Compare
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!
f228ac3
to
1968d08
Compare
Do you want to give it a shot, @tejasbubane? Otherwise let’s open a new issue so we don’t forget to implement it. |
1968d08
to
1b995cd
Compare
@bquorning I have added autocorrect. |
1b995cd
to
d137020
Compare
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
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.
All good.
Just in case maybe add a spec covering the case with semicolon?
d137020
to
184cba7
Compare
@pirj Added spec for semicolon - it is a little ugly though - since it does not remove the semicolon. |
No worries, it's less ugly than |
Closes #811
Before submitting the PR make sure the following are checked:
master
(if not - rebase it).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 created a new cop:
config/default.yml
.