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.

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

Implement SugarRecord.sum() #600

Merged
merged 3 commits into from
Jun 24, 2016
Merged

Implement SugarRecord.sum() #600

merged 3 commits into from
Jun 24, 2016

Conversation

SiebelsTim
Copy link
Contributor

analog to count()

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @whoshuu, @sibeliusseraphini and @RoyMontoya to be potential reviewers

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) to 71.415% when pulling 92b675e on SiebelsTim:impl-sum into ce7d802 on satyan:master.

@JonatanSalas
Copy link
Collaborator

@SiebelsTim Why don't you add some test in order to not decrease the coverage?

@SiebelsTim
Copy link
Contributor Author

@JonatanSalas At first I didn't find tests for count-like functions. After looking thoroughly, I added some tests :)

@SiebelsTim
Copy link
Contributor Author

Btw: Should I call toSQLNameDefault()around the field names, or should I let the user change it to underscores?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 72.849% when pulling f43255f on SiebelsTim:impl-sum into ce7d802 on satyan:master.

to use ... operator
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 72.849% when pulling 59a2a91 on SiebelsTim:impl-sum into ce7d802 on satyan:master.

@sibelius
Copy link
Contributor

LGTM

@sibelius sibelius merged commit 8a4cd58 into chennaione:master Jun 24, 2016
@sibelius
Copy link
Contributor

thanks

@SiebelsTim
Copy link
Contributor Author

Thanks :)

@SiebelsTim SiebelsTim deleted the impl-sum branch June 24, 2016 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants