-
Notifications
You must be signed in to change notification settings - Fork 160
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.
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
Add firebase analytics #525
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n-sdk into feat/Analytics
…se-kotlin-sdk into feat/Analytics
DenisPopkov
approved these changes
Jun 7, 2024
DenisPopkov
approved these changes
Jun 7, 2024
@nbransby this is ready for review now. Could you take a look at it when you have the time? |
DenisPopkov
approved these changes
Jun 11, 2024
@nbransby can this be merged? Since the android tests are still flaky :) |
nbransby
approved these changes
Jun 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains most of the firebase analytics implementation for Android, iOS and JS. This was requested in this recent issue and this very old issues.
For testing, this is very difficult. There is no real way to ensure logs arrived at the firebase side. There is no emulator for analytics and there is no response from the logEvent call to know if it succeeded. For now i only added a test that ensures the analytics object is available and makes sure it doesn't crash when calling the logEvent function. If you have ideas on how to better test this, please let me know!
To create the module i copy pasted one of the previously existing modules. I think i went through all the steps but im not 100% sure i did everything since there is no real guide for adding a new module. Could you let me know wether i added it in all the right places?