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

feat(dynamic-links): Firebase web V9 modular APIs #7296

Merged
merged 6 commits into from
Sep 5, 2023
Merged

feat(dynamic-links): Firebase web V9 modular APIs #7296

merged 6 commits into from
Sep 5, 2023

Conversation

exaby73
Copy link
Contributor

@exaby73 exaby73 commented Aug 17, 2023

Description

Related issues

Release Summary

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

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

@vercel
Copy link

vercel bot commented Aug 17, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 5, 2023 3:32am
react-native-firebase-next ❌ Failed (Inspect) Sep 5, 2023 3:32am

@exaby73 exaby73 mentioned this pull request Aug 17, 2023
Closed
8 tasks
@exaby73 exaby73 changed the title feat(dynamic-links): implemented V9 APIs feat(dynamic-links): implement V9 APIs Aug 17, 2023
@exaby73 exaby73 changed the title feat(dynamic-links): implement V9 APIs feat(dynamic-links): implemented V9 APIs Aug 17, 2023
@exaby73 exaby73 changed the title feat(dynamic-links): implemented V9 APIs feat(dynamic-links): V9 modular APIs Aug 17, 2023
@exaby73 exaby73 mentioned this pull request Aug 8, 2023
Closed
17 tasks
@exaby73 exaby73 marked this pull request as ready for review August 17, 2023 10:16
@exaby73 exaby73 force-pushed the feat/dynamic-links-v9-modular branch from 850fc26 to 0d3abed Compare August 22, 2023 07:50
@codecov
Copy link

codecov bot commented Sep 3, 2023 •
edited
Loading

Codecov Report

Merging #7296 (8585ac5) into main (309fec4) will increase coverage by 0.03%.
The diff coverage is 87.50%.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #7296      +/-   ##
============================================
+ Coverage     53.95%   53.97%   +0.03%     
  Complexity      736      736              
============================================
  Files           234      235       +1     
  Lines         11732    11740       +8     
  Branches       1876     1876              
============================================
+ Hits           6329     6336       +7     
- Misses         5054     5055       +1     
  Partials        349      349

russellwheatley
russellwheatley requested changes Sep 4, 2023
View reviewed changes
Copy link
Member

@russellwheatley russellwheatley left a comment

Choose a reason for hiding this comment

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

one small nit but LGTM

packages/dynamic-links/e2e/dynamicLinks.e2e.js Show resolved Hide resolved
@russellwheatley russellwheatley changed the title feat(dynamic-links): V9 modular APIs feat(dynamic-links): Firebase web V9 modular APIs Sep 4, 2023
russellwheatley
russellwheatley previously approved these changes Sep 4, 2023
View reviewed changes
mikehardy
mikehardy requested changes Sep 4, 2023
View reviewed changes
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.

some typing notes and param/return doc notes - this one is really close though I think

packages/dynamic-links/lib/modular/index.d.ts Outdated Show resolved Hide resolved
packages/dynamic-links/lib/modular/index.js Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
packages/dynamic-links/lib/modular/index.js Show resolved Hide resolved
@mikehardy mikehardy added the Workflow: Waiting for User Response Blocked waiting for user response. label Sep 4, 2023
mikehardy
mikehardy previously approved these changes Sep 4, 2023
View reviewed changes
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.

Hopefully this sails through CI - types look good now

@mikehardy mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Workflow: Waiting for User Response Blocked waiting for user response. labels Sep 4, 2023
@russellwheatley russellwheatley merged commit 573a4ee into main Sep 5, 2023
16 checks passed
@russellwheatley russellwheatley deleted the feat/dynamic-links-v9-modular branch September 5, 2023 08:39
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@russellwheatley russellwheatley russellwheatley approved these changes

@mikehardy mikehardy mikehardy left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@exaby73 @mikehardy @russellwheatley