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(fcm): Added support for specifying the analytics label for notifications. #597

Merged
merged 7 commits into from
Aug 12, 2019
Merged

feat(fcm): Added support for specifying the analytics label for notifications. #597

merged 7 commits into from
Aug 12, 2019
+159 −0

Conversation

chemidy
Copy link
Contributor

@chemidy chemidy commented Jul 20, 2019

@hiranya911 hiranya911 requested a review from egilmorez July 22, 2019 17:32
@hiranya911 hiranya911 assigned egilmorez and chemidy and unassigned egilmorez Jul 22, 2019
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
egilmorez
egilmorez reviewed Jul 22, 2019
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

Left some style comments. Thanks for the PR!

Cyrille Hemidy added 3 commits July 23, 2019 10:14
nit fix

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
aede523
Merge branch 'master' into analytics_label

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
a60d991
add validators

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e73f798
egilmorez
egilmorez approved these changes Jul 25, 2019
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

index.d.ts content looks good, thanks chemidy!

hiranya911
hiranya911 suggested changes Jul 25, 2019
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks good. Just need some test cases for the new validators.

And I'm also waiting to hear from the FCM team about fcmOptions vs fcm_options in the JSON payload.

hiranya911
hiranya911 approved these changes Aug 12, 2019
View reviewed changes
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM

@hiranya911 hiranya911 changed the title Analytics label feat(fcm): Added support for specifying the analytics label for notifications. Aug 12, 2019
@hiranya911 hiranya911 assigned hiranya911 and unassigned chemidy and chong-shao Aug 12, 2019
@hiranya911 hiranya911 merged commit d05dd23 into firebase:master Aug 12, 2019
@chemidy chemidy deleted the analytics_label branch August 15, 2019 01:51
@hiranya911 hiranya911 mentioned this pull request Aug 15, 2019
Closed
@parndt parndt mentioned this pull request Aug 21, 2019
Closed
@snyk-bot snyk-bot mentioned this pull request Oct 26, 2019
Open
This was referenced Nov 13, 2019
Closed
Closed
Closed
Closed
This was referenced Mar 10, 2020
Closed
Merged
@snyk-bot snyk-bot mentioned this pull request Apr 21, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chong-shao chong-shao

@hiranya911 hiranya911

@egilmorez egilmorez

Assignees

@hiranya911 hiranya911

Labels
release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@chemidy @hiranya911 @egilmorez @chong-shao