-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add section for redundant all
#340
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
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.
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 <koic.ito@gmail.com>
358c486 to
2159845
Compare
@koic
I've squashed the commits!
Thanks!
Related issue: #339
This PR adds "Redundant
all" section.