feat(fcm): Added support for specifying analytics label in notifications#89
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
hiranya911
left a comment
There was a problem hiding this comment.
Thanks @RannyRanny for putting this together. We need a couple of new types in this PR and test coverage.
…k for analytics label. Added tests.
|
@hiranya911 Added new classes and coveder tests |
hiranya911
left a comment
There was a problem hiding this comment.
Thanks @RannyRanny. Looks pretty good. Just a few changes/improvements needed. Then we can merge.
FirebaseAdmin/FirebaseAdmin/Messaging/Util/AnalyticsLabelChecker.cs
Outdated
Show resolved
Hide resolved
FirebaseAdmin/FirebaseAdmin/Messaging/Util/AnalyticsLabelChecker.cs
Outdated
Show resolved
Hide resolved
FirebaseAdmin/FirebaseAdmin/Messaging/Util/AnalyticsLabelChecker.cs
Outdated
Show resolved
Hide resolved
FirebaseAdmin/FirebaseAdmin/Messaging/Util/AnalyticsLabelChecker.cs
Outdated
Show resolved
Hide resolved
|
@hiranya911 Added requested changes |
hiranya911
left a comment
There was a problem hiding this comment.
Sorry, but it looks like this needs a few more updates. The main points are:
- Add the license header to all new source files.
- Make sure there's a newline at the end of each file.
AnalyticsLabelis optional, and therefore should allownull.
FirebaseAdmin/FirebaseAdmin/Messaging/Util/AnalyticsLabelChecker.cs
Outdated
Show resolved
Hide resolved
|
@egilmorez can you look at the documentation bits of this PR? |
|
@hiranya911 This is my first PR for open source. Can U please say when this will be available to use in NuGet package? |
|
@RannyRanny most likely next week. |
Solves #88