-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add section for redundant all
#340
Conversation
---- | ||
# bad | ||
User.all.order(:created_at) | ||
User.all.find(id) |
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.
Is it only applicable to the model or to a relation as well? ‘current_user.likes.all’?
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.
this applies to relation as well.
# bad
current_user.likes.all.order(:created_at)
# good
current_user.likes.order(:created_at)
However, it's uncertain whether this rule can be applied to current_user.likes.all
.
In RuboCop, determining whether likes
is a relation can be difficult since there are no Active Record query methods following all
. If likes
is not a relation, removing all
might change the result.
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.
True, examples of all
method followed by an AR query method could be added.
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.
True, examples of all method followed by an AR query method could be added.
Sorry for the confusion, but I want to make sure I understand your comment correctly.
Are you suggesting that I should add one or both of the following examples?
(like
is a relation of User)
relation.all.ar_query_method
# bad
current_user.likes.all.order(:created_at)
# good
current_user.likes.order(:created_at)
relation.ar_query_method.all
# bad
current_user.likes.order(:created_at).all
# good
current_user.likes.order(:created_at)
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.
I imagined it as 1. Either way, the receiver is an ActiveRecord_Relation
object.
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 for your comment. I've added some examples.
5d906af
to
8661a42
Compare
@masato-bkn Can you squash your commits into one? |
Co-authored-by: Koichi ITO <[email protected]>
358c486
to
2159845
Compare
@koic |
Thanks! |
Related issue: #339
This PR adds "Redundant
all
" section.