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.

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

fix(app, android): firebase-android-sdk 29.0.3 to fix underlying NPE in 29.0.2 #5946

Merged
merged 5 commits into from
Dec 18, 2021

Conversation

kenMarquez
Copy link
Contributor

@kenMarquez kenMarquez commented Dec 17, 2021

Description

I'm updating the latest version of this library in our app so we can collect ANR issues using the latest version of Firebase Crashlytics, so when reading the Firebase Android BoM documentation I saw that they sent a patch yesterday to avoid a crash (NullPointerException) with the previous version of the firebase analytics library:
https://issuetracker.google.com/issues/210908929

So I think it will be wonderful to have the most stable version. 👍🏽

image

Release Summary

  • firebase-android-sdk 29.0.3
  • Add ANR collection support documentation

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

No breaking changes, only internal changes in the android firebase analytics library

Think react-native-firebase is great? Please consider supporting the project with any of the below:

🔥

@vercel
Copy link

vercel bot commented Dec 17, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/HQxGHfrTiRfa94XvnaUkFSCk3Ugh
✅ Preview: https://react-native-firebase-git-fork-kenmarquez-main-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/EYTEfeyUgFR12f4Eq7fQEKLN4v9F
✅ Preview: Canceled

[Deployment for 804c16a canceled]

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2021

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next December 17, 2021 22:34 Inactive
@kenMarquez kenMarquez changed the title feat(deps) firebase-android-sdk 29.0.3 feat(deps): firebase-android-sdk 29.0.3 Dec 17, 2021
mikehardy
mikehardy previously approved these changes Dec 17, 2021
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This is great, thanks! DIdn't realize they had a crash bug in 29.0.2 - they don't do formal releases on github so there is nothing to "watch" and I don't get notified 😅 - until I release this, please know that you can a) use the patch-package set from the actions here or b) you can do the override in your build.gradle to make sure you don't have any NullPointerExceptions - https://rnfirebase.io/#android

I'm going to edit the title to just be a fix though, it's only a patch release from them, should only be a patch release from us :-). Cheers!

@mikehardy mikehardy changed the title feat(deps): firebase-android-sdk 29.0.3 fix(app, android): firebase-android-sdk 29.0.3 to fix underlying NPE in 29.0.2 Dec 17, 2021
@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Dec 17, 2021
@codecov
Copy link

codecov bot commented Dec 17, 2021

Codecov Report

Merging #5946 (532249c) into main (a5a8270) will decrease coverage by 0.36%.
The diff coverage is n/a.

❗ Current head 532249c differs from pull request most recent head 804c16a. Consider uploading reports for the commit 804c16a to get more accurate results

@@             Coverage Diff              @@
##               main    #5946      +/-   ##
============================================
- Coverage     53.04%   52.68%   -0.35%     
  Complexity      622      622              
============================================
  Files           208      208              
  Lines         10171    10171              
  Branches       1618     1618              
============================================
- Hits           5394     5358      -36     
- Misses         4523     4556      +33     
- Partials        254      257       +3     

@mikehardy
Copy link
Collaborator

Even for small fixes we need the CLA signed if you could? The details link on the CLA comment + action have more info on how

@kenMarquez
Copy link
Contributor Author

kenMarquez commented Dec 18, 2021

@mikehardy Yes of course! The CLA is already added it!

I see the Documentation / Spelling & Grammar job failed, i think the documentation is having an issue with the word ANR ,

Adding this word in the .spellcheck.dict.txt file should work, right?

@mikehardy
Copy link
Collaborator

Yep, adding it to the spelling dictionary sounds perfect

@kenMarquez
Copy link
Contributor Author

Done @mikehardy, I am really amazed by all the checks you have set up in the library,
congratulations! is an amazing job

@mikehardy
Copy link
Collaborator

Awesome thanks. And yeah, quality is really really important for a library this fundamental. The automation is the only way to have confidence we maintain the level we expect while making changes

@mikehardy mikehardy merged commit 051f4a6 into invertase:main Dec 18, 2021
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Dec 18, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants