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): minSdk should be 19 to match firebase-android-sdk #5984

Merged
merged 1 commit into from
Jan 3, 2022
Merged

fix(app, android): minSdk should be 19 to match firebase-android-sdk #5984

merged 1 commit into from
Jan 3, 2022

Conversation

SaeedZhiany
Copy link
Contributor

Description

Related issues

As @mikehardy requested in this comment I have tested react-native-firebase on projects with minSdk 19 and it works fine for me.

Release Summary

Downgraded default minSdk from 21 to 19 to support more android devices.

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

It worth mentioning that I'm currently using the cloud messaging package in my project. if anyone is willing to test this PR on the other packages, feel free to clean its project and then apply the same simple change on this PR, and test their packages' functionality, and report the result on this Thread.


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

@vercel
Copy link

vercel bot commented Jan 3, 2022

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/GMKtZAZ7VEF6BfXPLYU6MQhybqZo
✅ Preview: https://react-native-firebase-git-fork-saeedzhiany-patch-1-invertase.vercel.app

react-native-firebase-next – ./website_modular

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

[Deployment for c1f38ea canceled]

@codecov
Copy link

codecov bot commented Jan 3, 2022

Codecov Report

Merging #5984 (4970301) into main (432a7bd) will increase coverage by 27.51%.
The diff coverage is n/a.

❗ Current head 4970301 differs from pull request most recent head c1f38ea. Consider uploading reports for the commit c1f38ea to get more accurate results

@@              Coverage Diff              @@
##               main    #5984       +/-   ##
=============================================
+ Coverage     25.45%   52.95%   +27.51%     
- Complexity        0      622      +622     
=============================================
  Files            97      208      +111     
  Lines          4245    10189     +5944     
  Branches       1033     1619      +586     
=============================================
+ Hits           1080     5395     +4315     
- Misses         2571     4540     +1969     
+ Partials        594      254      -340     

@SaeedZhiany SaeedZhiany changed the title Downgraded default minSdk from 21 to 19 fix(app,android): Downgraded default minSdk from 21 to 19 Jan 3, 2022
@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next January 3, 2022 11:49 Inactive
@mikehardy mikehardy changed the title fix(app,android): Downgraded default minSdk from 21 to 19 fix(app, android): minSdk should be 19 to match firebase-android-sdk Jan 3, 2022
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.

Nice! Thank you. I think for this one it simply wouldn't build if it was not going to work, and this matches the documented min sdk of firebase-android-sdk so should be fine

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Jan 3, 2022
@mikehardy mikehardy merged commit 8015779 into invertase:main Jan 3, 2022
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Jan 3, 2022
@SaeedZhiany SaeedZhiany deleted the patch-1 branch January 4, 2022 04:52
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.

2 participants