-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Fix #6931] Layout/AccessModifierIndentation
outdent style now ignores modifiers with parameters
#6935
[Fix #6931] Layout/AccessModifierIndentation
outdent style now ignores modifiers with parameters
#6935
Conversation
Layout/AccessModifierIndentation
outdent style now ignores modifiers with parameters
The change looks good to me. Can you confirm it's consistent with how they do things in Rails? |
One more thing - you can also add some test cases of a modifier being used directly with |
Thanks for having a look @bbatsov! Rails doesn't use this cop. They do have Regarding the new tests, I'll add them. Should I also rename the cop? |
No, I'd rather just update the cop documentation to be more explicit about its scope and what's outside of it. |
I tried to clarify the docs a bit, and added a changelog entry. I tagged it as a bug fix. The alternative would be to tag this as a change and change the |
It's fine as it is. Thanks! |
Great, thanks! |
7151: Bump rubocop to 0.68.0 r=hsbt a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that our rubocop version is getting outdated. Also, I added [a PR to rubocop](rubocop/rubocop#6935) so that we can remove the exclusions in our configuration, and it has now been released. ### What is your fix for the problem, implemented in this PR? My fix is to update rubocop to the latest version, update the configuration, and fix new offenses. Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
7151: Bump rubocop to 0.68.0 r=hsbt a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that our rubocop version is getting outdated. Also, I added [a PR to rubocop](rubocop/rubocop#6935) so that we can remove the exclusions in our configuration, and it has now been released. ### What is your fix for the problem, implemented in this PR? My fix is to update rubocop to the latest version, update the configuration, and fix new offenses. Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
This is the fix I propose for #6931.
It doesn't include the remane of
Layout/AccessModifierIndentation
toLayout/BareAccessModifierIndentation
though. I can add that if needed.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).and description in grammatically correct, complete sentences.
bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.