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

feat(ios): UMP Consent on new arch #667

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Conversation

dylancom
Copy link
Collaborator

@dylancom dylancom commented Dec 4, 2024

Description

UMP Consent on new arch

Note that importing types does (still) not work with codegen, so I moved the types to the spec directly.

Any folks that can test this? @TaylorDale @wjaykim @DoctorJohn @birdofpreyru

Copy link

docs-page bot commented Dec 4, 2024

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/react-native-google-mobile-ads~667

Documentation is deployed and generated using docs.page.

@dylancom dylancom force-pushed the feature/ump-on-new-arch branch from 764362a to e16bb7b Compare December 4, 2024 11:46
@dylancom dylancom requested a review from mikehardy December 4, 2024 11:47
@dylancom dylancom force-pushed the feature/ump-on-new-arch branch from e546cf5 to def14f4 Compare December 4, 2024 12:01
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.

Curious about the async-ness on the type signatures but can't see anything truly wrong
Testing always good though if anyone else has time

@dylancom
Copy link
Collaborator Author

dylancom commented Dec 5, 2024

Curious about the async-ness on the type signatures but can't see anything truly wrong

Testing always good though if anyone else has time

Still async. But there is the possibility now to make some methods sync for new arch. Not sure what path we should take there, introduce new methods / breaking change? Send it back sync on the new arch, but wrap it in a promise for now...

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.

I don't think changing it to sync and taking a break really has any value for people personally, these are things people are doing on startup (where maybe, just maybe async isn't available)

@dylancom dylancom merged commit 19e9379 into main Dec 9, 2024
12 checks passed
@dylancom dylancom deleted the feature/ump-on-new-arch branch December 9, 2024 10:09
@mikehardy
Copy link
Collaborator

🎉 This PR is included in version 14.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants