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

Adding a new API that allows iOS users to log notification delivery signal that exports to BigQuery. #6181

Merged
merged 44 commits into from
Jul 27, 2021
Merged

Adding a new API that allows iOS users to log notification delivery signal that exports to BigQuery. #6181

merged 44 commits into from
Jul 27, 2021

Conversation

charlotteliang
Copy link
Contributor

@charlotteliang charlotteliang commented Jul 31, 2020 •
edited
Loading

We got approved within our team to proceed with this launch. Please review this again, the API review is approved at
go/fm-ios-delivery

@maksymmalyhin
Copy link
Contributor

@chliangGoogle I feel like there must be a LaunchCall associated with the feature. I think it would be useful to link it to the PR somehow.

@charlotteliang
Copy link
Contributor Author

I'm still struggling with the nanopb encoding, once the code is ready for review, I will kick off a launchcal.

@paulb777
Copy link
Member

paulb777 commented Oct 9, 2020

@chliangGoogle Is this PR still relevant? If not please close.

@charlotteliang
Copy link
Contributor Author

charlotteliang commented Oct 12, 2020 •
edited
Loading

@paulb777 It is blocked on internal PM review.

@google-oss-bot
Copy link

google-oss-bot commented Jun 2, 2021 •
edited
Loading

Coverage Report

Affected SDKs

  • FirebaseMessaging-iOS-FirebaseMessaging.framework

    SDK overall coverage changed from 64.35% (04ab074) to 64.60% (d45e2fe) by +0.25%.

    Filename Base (04ab074) Head (d45e2fe) Diff
    FIRMessagingContextManagerService.m 66.83% 66.99% +0.16%
    FIRMessagingExtensionHelper.m 82.68% 85.91% +3.23%
    FIRMessagingTokenManager.m 42.92% 42.15% -0.77%

Test Logs

@charlotteliang charlotteliang changed the title [draft] Using Firelog to log delivery metrics for Messaging Adding a new API that allows iOS users to log notification delivery signal that exports to BigQuery. Jun 2, 2021
paulb777
paulb777 reviewed Jun 3, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

This needs a rebase to the latest master and tests passing.

FirebaseMessaging.podspec Outdated Show resolved Hide resolved
maksymmalyhin
maksymmalyhin approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@maksymmalyhin maksymmalyhin left a comment

Choose a reason for hiding this comment

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

LGTM, with a couple minor comments/questions.

FirebaseMessaging/ProtoSupport/proto_generator.py Outdated Show resolved Hide resolved
FirebaseMessaging/ProtoSupport/proto_generator.py Show resolved Hide resolved
FirebaseMessaging/ProtoSupport/proto_generator.py Outdated Show resolved Hide resolved
FirebaseMessaging/Sources/FIRMessagingConstants.m Show resolved Hide resolved
FirebaseMessaging/Sources/FIRMessagingExtensionHelper.m Show resolved Hide resolved
FirebaseMessaging/Sources/FIRMessagingExtensionHelper.m Show resolved Hide resolved
ncooke3
ncooke3 reviewed Jun 7, 2021
View reviewed changes
FirebaseMessaging.podspec Outdated Show resolved Hide resolved
paulb777
paulb777 reviewed Jun 7, 2021
View reviewed changes
FirebaseMessaging.podspec Outdated Show resolved Hide resolved
FirebaseMessaging.podspec Outdated Show resolved Hide resolved
FirebaseMessaging/ProtoSupport/generate_protos.sh Outdated Show resolved Hide resolved
paulb777
paulb777 reviewed Jul 21, 2021
View reviewed changes
FirebaseMessaging.podspec Outdated Show resolved Hide resolved
FirebaseMessaging/ProtoSupport/Protos/me.options Outdated Show resolved Hide resolved
FirebaseMessaging/Sources/Protogen/nanopb/me.nanopb.c Outdated Show resolved Hide resolved
FirebaseMessaging/Sources/Protogen/nanopb/me.nanopb.c Outdated Show resolved Hide resolved
@charlotteliang charlotteliang merged commit ce87d25 into master Jul 27, 2021
@charlotteliang charlotteliang deleted the fm-firelog branch July 27, 2021 18:16
maksymmalyhin added a commit that referenced this pull request Jul 29, 2021
@maksymmalyhin
Revert " Adding a new API that allows iOS users to log notification d…
dffc37a 
…elivery signal that exports to BigQuery. (#6181)"

This reverts commit ce87d25.
@maksymmalyhin maksymmalyhin mentioned this pull request Jul 29, 2021
Merged
maksymmalyhin added a commit that referenced this pull request Jul 29, 2021
@maksymmalyhin
Revert "Adding a new API that allows iOS users to log notification de…
b8e974d 
…livery signal that exports to BigQuery." (#8458)

* Revert "Prevent firelog from logging invalid data (#8443)"

This reverts commit 7fc7325.

* Revert "Update changelog (#8440)"

This reverts commit ecc7d6d.

* Revert " Adding a new API that allows iOS users to log notification delivery signal that exports to BigQuery. (#6181)"

This reverts commit ce87d25.
@firebase firebase locked and limited conversation to collaborators Aug 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@paulb777 paulb777 paulb777 left review comments

@ncooke3 ncooke3 ncooke3 left review comments

@github-actions GitHub Actions github-actions left review comments

@maksymmalyhin maksymmalyhin maksymmalyhin approved these changes

@visumickey visumickey Awaiting requested review from visumickey

@ryanwilson ryanwilson Awaiting requested review from ryanwilson

Assignees
No one assigned
Labels
api: messaging cla: yes public-api-change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@charlotteliang @maksymmalyhin @paulb777 @google-oss-bot @ncooke3 @googlebot