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): remove firebase-core from dependencies #4597

Merged
merged 1 commit into from
Nov 23, 2020

Conversation

rnike
Copy link
Contributor

@rnike rnike commented Nov 23, 2020

Description

According to the firebase release notes, the firebase-core for Android is no longer needed.

Screen Shot 2020-11-23 at 5 50 07 PM

Related issues

Fixes #4593

Release Summary

Remove firebase-core from Android dependencies.

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

  • Run tests:android:build

截圖 2020-11-23 下午10 48 44

  • Build and run on our app, based on react-native-firebase v10.0.0, including packages:
  • @react-native-firebase/analytics
  • @react-native-firebase/app
  • @react-native-firebase/crashlytics
  • @react-native-firebase/messaging
  • @react-native-firebase/perf

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

@CLAassistant
Copy link

CLAassistant commented Nov 23, 2020

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Nov 23, 2020

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

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/cq2irg8l7
✅ Preview: Failed

[Deployment for 6873d39 failed]

@codecov
Copy link

codecov bot commented Nov 23, 2020

Codecov Report

Merging #4597 (3f10237) into master (c36200b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4597   +/-   ##
=======================================
  Coverage   88.98%   88.98%           
=======================================
  Files         109      109           
  Lines        3710     3710           
  Branches      346      346           
=======================================
  Hits         3301     3301           
  Misses        368      368           
  Partials       41       41           

@rnike rnike force-pushed the android/remove-firebase-core branch from 83362aa to 6873d39 Compare November 23, 2020 14:57
@rnike rnike changed the title build(app, android): remove firebase-core from dependencies feat(app, android): remove firebase-core from dependencies Nov 23, 2020
@mikehardy
Copy link
Collaborator

Fantastic, thanks for this! The Vercel CI failure is unrelated, it should resolve when merged, but either way won't block this, and the other main CI jobs (android + ios E2E) have already passed the point they would have failed with this

We will need the CLA signed (just follow the details link on the CI check for it) in order to merge, but it's a 5 second job

Noted from the linked issue as well as here: thanks for testing this locally, that plus the CI here gives me confidence that it's going to work. It's a semi-fundamental change but that also means it should have failed fast with at least one of the tests you and CI have done

Cheers

@mikehardy mikehardy changed the title feat(app, android): remove firebase-core from dependencies fix(app, android): remove firebase-core from dependencies Nov 23, 2020
@rnike
Copy link
Contributor Author

rnike commented Nov 23, 2020

@mikehardy Thanks for the instruction, the CLA is signed.

@mikehardy mikehardy merged commit 22c615c into invertase:master Nov 23, 2020
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Firebase Android library firebase-core is no longer needed.
3 participants