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

firestore: add LimitToLast samples #1599

Merged
merged 21 commits into from Jul 30, 2020
Merged

firestore: add LimitToLast samples #1599

merged 21 commits into from Jul 30, 2020

Conversation

IlyaFaer
Copy link
Contributor

@IlyaFaer IlyaFaer commented Jul 22, 2020 •
edited

Requires the feature PR to be merged and released
Closes #1598

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 22, 2020
@tbpg tbpg changed the title add LimitToLast samples firestore: add LimitToLast samples Jul 22, 2020
@IlyaFaer IlyaFaer marked this pull request as ready for review July 28, 2020 07:31
@IlyaFaer
Copy link
Contributor Author

@codyoss, @tritone, please, review

tritone
tritone reviewed Jul 28, 2020
View reviewed changes
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

Looks basically fine given that the snippets in this file do not follow the canonical sample. I see that none of them are under test; do we want to add a simple test to exercise them?

Also, you'll have to pull in firestore/v1.3.0 in order for the CI to pass.

Gurov Ilya and others added 2 commits July 29, 2020 10:59
@BenWhitehead
Update firestore/firestore_snippets/query.go
2556c61 
Co-authored-by: BenWhitehead <BenWhitehead@users.noreply.github.com>
Merge branch 'master' into limit_to_last_samples
066ad46
@tritone
Copy link
Contributor

tritone commented Jul 29, 2020

Talked with @tbpg and the automated dependency update is blocked currently. @IlyaFaer could you update go.mod to use firestore/v1.3.0 and add the change to this PR?

@tbpg
Copy link
Contributor

tbpg commented Jul 29, 2020

You can update the go.mod with:

go get cloud.google.com/go/firestore@v1.3.0
go mod tidy

@IlyaFaer
Copy link
Contributor Author

@tritone, @tbpg, not sure how to deal with it. Sorry, I've never changed dependencies in Go manually. I've thought it's easy as renovate bot PR, but looks like I'm missing something

@IlyaFaer
Copy link
Contributor Author

It's working fine locally (e.g. this commit), but when I'm pushing it, kokoro fails

Безымянный

IlyaFaer and others added 2 commits July 30, 2020 12:25
@tbpg tbpg added automerge Merge the pull request once unit tests and other checks pass. kokoro:run Add this label to force Kokoro to re-run the tests. labels Jul 30, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 30, 2020
@tbpg tbpg merged commit d3cafb3 into GoogleCloudPlatform:master Jul 30, 2020
@tbpg tbpg removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 30, 2020
@IlyaFaer IlyaFaer deleted the limit_to_last_samples branch July 31, 2020 07:32
This was referenced Aug 27, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tritone tritone tritone left review comments

@BenWhitehead BenWhitehead BenWhitehead approved these changes

@tbpg tbpg tbpg approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

firestore: add LimitToLast samples
5 participants
@IlyaFaer @tritone @tbpg @BenWhitehead @kokoro-team