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

feat: SUM / AVG #1218

Merged
merged 14 commits into from
Apr 24, 2023
Merged

feat: SUM / AVG #1218

merged 14 commits into from
Apr 24, 2023

Conversation

ehsannas
Copy link
Contributor

@ehsannas ehsannas commented Mar 1, 2023

No description provided.

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: firestore Issues related to the googleapis/java-firestore API. labels Mar 1, 2023
@ehsannas ehsannas marked this pull request as ready for review March 1, 2023 23:45
@ehsannas ehsannas requested a review from a team as a code owner March 1, 2023 23:45
@ehsannas ehsannas requested review from MarkDuckworth and removed request for a team March 1, 2023 23:45
MarkDuckworth
MarkDuckworth reviewed Mar 4, 2023
View reviewed changes
Copy link
Contributor

@MarkDuckworth MarkDuckworth left a comment

Choose a reason for hiding this comment

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

Overall this looks great! There are a few comments for your consideration. I have a few test cases to research and follow up on.

google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java Outdated Show resolved Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java Show resolved Hide resolved
google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuerySnapshot.java Outdated Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java Outdated Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Outdated Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Show resolved Hide resolved
google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java Outdated Show resolved Hide resolved
@MarkDuckworth MarkDuckworth added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 14, 2023
@MarkDuckworth MarkDuckworth removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 24, 2023
@ehsannas ehsannas merged commit 227b818 into ehsann/sum-average Apr 24, 2023
@ehsannas ehsannas deleted the ehsann/sum-average-2 branch April 24, 2023 16:51
ehsannas added a commit that referenced this pull request Aug 2, 2023
@ehsannas
feat: SUM / AVG (#1218)
7cf1c86 
* feat: SUM / AVG (real files)

* Remove aggregate field duplicates (if any).

* Clean up and fixes.

* Clean up comments, and add Nonnull where possible.

* Add more public docs.

* More cleanup.

* Update hashCode and equals for AggregateQuery.

* Address code review comments. more to come.

* fix test name.

* Better comment.

* Fix alias encoding.

* Remove TODO.

* Revert the way alias is constructed.

* Backport test updates.
ehsannas added a commit that referenced this pull request Aug 2, 2023
@ehsannas
feat: SUM / AVG (#1218)
1a749c9 
* feat: SUM / AVG (real files)

* Remove aggregate field duplicates (if any).

* Clean up and fixes.

* Clean up comments, and add Nonnull where possible.

* Add more public docs.

* More cleanup.

* Update hashCode and equals for AggregateQuery.

* Address code review comments. more to come.

* fix test name.

* Better comment.

* Fix alias encoding.

* Remove TODO.

* Revert the way alias is constructed.

* Backport test updates.

fix format.
ehsannas added a commit that referenced this pull request Aug 2, 2023
@ehsannas
feat: SUM / AVG (#1218)
8cde315 
* feat: SUM / AVG (real files)

* Remove aggregate field duplicates (if any).

* Clean up and fixes.

* Clean up comments, and add Nonnull where possible.

* Add more public docs.

* More cleanup.

* Update hashCode and equals for AggregateQuery.

* Address code review comments. more to come.

* fix test name.

* Better comment.

* Fix alias encoding.

* Remove TODO.

* Revert the way alias is constructed.

* Backport test updates.

fix format.

fix import stmt.
ehsannas added a commit that referenced this pull request Oct 9, 2023
@ehsannas @gcf-owl-bot
feat: Sum and Average aggregations (#1387)
afa5c01 
* feat: SUM / AVG (#1218)

* feat: SUM / AVG (real files)

* Remove aggregate field duplicates (if any).

* Clean up and fixes.

* Clean up comments, and add Nonnull where possible.

* Add more public docs.

* More cleanup.

* Update hashCode and equals for AggregateQuery.

* Address code review comments. more to come.

* fix test name.

* Better comment.

* Fix alias encoding.

* Remove TODO.

* Revert the way alias is constructed.

* Backport test updates.

fix format.

fix import stmt.

* feat: Add long alias support for aggregations. (#1267)

* feat: Add long alias support for aggregations.

* address comments.

* Better method name and replace hardcoded "count" with "aggregate_0".

* Remove duplicate aggregations.

* add static import.

* Fix tests. All tests pass.

* Address comments.

* Improve the documentation to match strongly typed languages.

* Do not use wildcard import.

* Do not use wildcard import (2).

* Do not use wildcard import (3).

* Do not use wildcard import (4).

* Fix the javadoc.

* Add license header, and remove unused test code.

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* better regex.

* Add more tests for cursors.

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
cherylEnkidu pushed a commit that referenced this pull request Dec 11, 2023
@ehsannas @gcf-owl-bot @cherylEnkidu
feat: Sum and Average aggregations (#1387)
0d2a380 
* feat: SUM / AVG (#1218)

* feat: SUM / AVG (real files)

* Remove aggregate field duplicates (if any).

* Clean up and fixes.

* Clean up comments, and add Nonnull where possible.

* Add more public docs.

* More cleanup.

* Update hashCode and equals for AggregateQuery.

* Address code review comments. more to come.

* fix test name.

* Better comment.

* Fix alias encoding.

* Remove TODO.

* Revert the way alias is constructed.

* Backport test updates.

fix format.

fix import stmt.

* feat: Add long alias support for aggregations. (#1267)

* feat: Add long alias support for aggregations.

* address comments.

* Better method name and replace hardcoded "count" with "aggregate_0".

* Remove duplicate aggregations.

* add static import.

* Fix tests. All tests pass.

* Address comments.

* Improve the documentation to match strongly typed languages.

* Do not use wildcard import.

* Do not use wildcard import (2).

* Do not use wildcard import (3).

* Do not use wildcard import (4).

* Fix the javadoc.

* Add license header, and remove unused test code.

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* better regex.

* Add more tests for cursors.

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cherylEnkidu cherylEnkidu cherylEnkidu left review comments

@MarkDuckworth MarkDuckworth MarkDuckworth approved these changes

Assignees

@MarkDuckworth MarkDuckworth

Labels
api: firestore Issues related to the googleapis/java-firestore API. size: xl Pull request size is extra large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ehsannas @MarkDuckworth @cherylEnkidu