Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Relation#count doesn't use options anymore. #12518

Merged
merged 2 commits into from
Oct 13, 2013
Merged

Relation#count doesn't use options anymore. #12518

merged 2 commits into from
Oct 13, 2013

Conversation

vipulnsward
Copy link
Member

Relation#count doesn't use options anymore.

This is after- cd87c85

@rafaelfranca
Copy link
Member

We need to check the others method on Calculation. For example perform_calculation is taking option and it is not using. We need to check if minimum, maximum still accept any kind of options too.

@vipulnsward
Copy link
Member Author

@rafaelfranca Removed form all places.

rafaelfranca
rafaelfranca reviewed Oct 13, 2013
View reviewed changes
activerecord/test/cases/relations_test.rb
@@ -295,7 +295,7 @@ def test_null_relation_content_size_methods
def test_null_relation_calculations_methods
assert_no_queries do
assert_equal 0, Developer.none.count
assert_equal 0, Developer.none.calculate(:count, nil, {})
assert_equal 0, Developer.none.calculate(:count, nil )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra whitespaces

@vipulnsward
Stop accepting options for Relation#average, Relation#minimum, …
42d3d3c 
…`Relation#maximum`, `Relation#calculate`, `perform_calculation`, `NullRelation#calculate` as they isn't used anymore.
rafaelfranca added a commit that referenced this pull request Oct 13, 2013
@rafaelfranca
Merge pull request #12518 from vipulnsward/remove_count_options
dcff027 
`Relation#count` doesn't use options anymore.
@rafaelfranca rafaelfranca merged commit dcff027 into rails:master Oct 13, 2013
@vipulnsward
Copy link
Member Author

@rafaelfranca Thanks!

rafaelfranca added a commit that referenced this pull request Dec 11, 2013
@rafaelfranca
Revert "Merge pull request #12518 from vipulnsward/remove_count_options"
e1ce005 
It is needed for activerecord-depecated_finders

This reverts commit dcff027, reversing
changes made to 3a20939.
@vipulnsward vipulnsward deleted the remove_count_options branch February 22, 2016 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
activerecord
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vipulnsward @rafaelfranca