-
-
Notifications
You must be signed in to change notification settings - Fork 263
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 UniqBeforePluck cop #135
Conversation
fdaa312
to
c727652
Compare
@santib Sorry for the late reply.
First of all, I think that the current implementation is good to prevent the error.
Yeah, certainly breaking change is not desirable. It's a bit weird, but let's keep the cop name as Can you rebase with the latest master branch and add the changelog entry? |
6bbf863
to
81ce30d
Compare
@koic updated. I put it in the "bug fixes" section since it could be considered a bug for newer versions of rails, but let me know if you want to change that or something else |
81ce30d
to
2321b87
Compare
@santib Thank you for the updating. Can you add |
0d01153
to
012f2e6
Compare
@koic done :) |
legacy-docs/cops_rails.md
Outdated
|
||
# good | ||
Model.uniq.pluck(:id) | ||
Model.distinct.pluck(: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.
The document has been changed from Markdown (legacy-docs/cops_rails.md) to AsciiDoc (docs/modules/ROOT/pages/cops_rails.adoc). So, this Markdown document is not maintained.
Can you revert this Markdown doc?
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.
oops, not sure how this doc got updated 🤔
012f2e6
to
8460d92
Compare
Nicely done, Thank you! |
Follow up rubocop#135. `Model.pluck(:id).distinct` happens the following `NoMethodError`. ``` NoMethodError: undefined method `distinct' for [42]:Array ``` This PR replaces it with `Array#uniq`.
Resolves #91
Notes:
uniq
fordistinct
or if it's fine to convertModel.pluck(:id).uniq
toModel.distinct.pluck(:id)
within this cop.DistinctBeforePluck
or not, since that would be a breaking change.