Skip to content
This repository was archived by the owner on Dec 30, 2024. It is now read-only.

Add code samples for Remote Config operations #9

Merged
merged 3 commits into from
Dec 15, 2020
Merged

Conversation

lahirumaramba
Copy link
Member

@samtstern
Copy link
Contributor

@lahirumaramba LGTM, I assume the CI failures are because the SDK has not released yet? Please wait until CI is green to merge these.

Copy link
Contributor

@kroikie kroikie left a comment

Choose a reason for hiding this comment

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

LGTM

@lahirumaramba
Copy link
Member Author

lahirumaramba commented Dec 3, 2020

@lahirumaramba LGTM, I assume the CI failures are because the SDK has not released yet? Please wait until CI is green to merge these.

@samtstern Thanks! Yes, the current SDK the project is pointing to does not have the new RC features, hence the CI failures. Once the SDK is released, do you think I should update admin/build.gradle as well? If so, I could include that change in the same PR.

@samtstern
Copy link
Contributor

samtstern commented Dec 3, 2020

@lahirumaramba yep exactly, if you bump the dependency in this PR as well then it will be green before you merge.

If these snippets have to appear in docs via includecode then you can have the Tech Writer use git_revision="lm-rc-snippets" to stage from your branch and then make a note to remove that after launch.

@egilmorez
Copy link
Contributor

Sam's right about git_revision, but it comes with a caveat: you can't use the newer, registry-based named snippets with that flag. So I'd resist that unless there's a big rush.

@samtstern
Copy link
Contributor

@egilmorez actually git_revision is totally compatible with the Snippet Registry and there's even a refactoring workflow to undo it!
https://g3doc.corp.google.com/devrel/tools/includecode/g3doc/guide-refactor-snippets.md?cl=head#option-2-migrate-branches

@egilmorez
Copy link
Contributor

Check you guys out! I'll try that today; I'm an excellent test for ease-of-use criteria :D

@lahirumaramba
Copy link
Member Author

@samtstern @hiranya911 :
Updated admin/build.gradle to point to the latest firebase-admin v7.1.0. Had to update Notification to use the builder() in FirebaseMessagingSnippets. Let me know if you would rather have us done that in a separate PR (the upgrade and changes to FirebaseMessagingSnippets) and then follow up with this PR.
Thank you!

@samtstern samtstern merged commit ea3c05d into master Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants