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: 6078 update imports #197

Merged
merged 6 commits into from
Mar 12, 2024
Merged

fix: 6078 update imports #197

merged 6 commits into from
Mar 12, 2024

Conversation

BrandonStalnaker
Copy link
Contributor

@BrandonStalnaker BrandonStalnaker commented Feb 28, 2024

Summary

Users seem to be having trouble figuring out the correct imports and/or React keeps changing the system and causing them to need to change there tactics. This change ensures we are using a more durable way of importing mParticle.

Additionally, with recent architecture changes from React Native further configuration needs done at the framework level depending on what version of the architecture is being used. This change updates the react-native-mparticle.podspec to consume the install_modules_dependencies function provided by React Native to configure the pods dependencies. Similar to whats being done on React's frameworks.

Testing Plan

Tested by making a react app from scratch and confirming it ran and could send data to mParticle. Can't fully confirm until cocaopods is updated.

Master Issue

Closes https://go.mparticle.com/work/SQDSDKS-6078

Copy link

@einsteinx2 einsteinx2 left a comment

Choose a reason for hiding this comment

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

Readme update and header changes look good. Podspec seems ok but I'm getting npm errors when doing a pod lint which will need to succeed to release this.

Ideally let's add another step to the pull requests actions to do a pod lint for iOS.

Also we need to update all checkout@v3 actions to v4 for github runner reasons.

@BrandonStalnaker BrandonStalnaker force-pushed the fix/6078-Update-Imports branch from a58c30a to 55d9e64 Compare March 7, 2024 14:24
@BrandonStalnaker BrandonStalnaker force-pushed the fix/6078-Update-Imports branch from 76cced9 to b5ac0ff Compare March 12, 2024 14:55
Copy link
Member

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

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

LGTM. Tested locally with all these changes

@BrandonStalnaker BrandonStalnaker merged commit db8e980 into main Mar 12, 2024
10 checks passed
@BrandonStalnaker BrandonStalnaker deleted the fix/6078-Update-Imports branch March 12, 2024 14:59
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