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

Remove TARGET_DEVICE_PLATFORM_NAME matching from Info.plist touching #2936

Conversation

sebastianv1
Copy link
Contributor

It appears that the TARGET_DEVICE_PLATFORM_NAME check didn't account for iphonesimulator platform name which was causing unit tests running on a test host to not reflect changes made in source code.

@sebastianv1 sebastianv1 requested a review from a team as a code owner March 7, 2024 19:46
@brentleyjones
Copy link
Contributor

Hmm, this was specifically set to that because the change was only needed to fix a device deployment issue. Are you saying that the same issue exists on the simulator?

@brentleyjones brentleyjones changed the title Fix TARGET_DEVICE_PLATFORM_NAME matching Include simulator in TARGET_DEVICE_PLATFORM_NAME matching Mar 7, 2024
@brentleyjones
Copy link
Contributor

Does this issue also happen for tvOS? Maybe we need to remove the TARGET_DEVICE_PLATFORM_NAME check all together?

@sebastianv1
Copy link
Contributor Author

@brentleyjones Thats correct; it appears to happen on simulator as well. Devs have been reporting that source code changes aren’t reflected in test host target apps.

@sebastianv1
Copy link
Contributor Author

sebastianv1 commented Mar 7, 2024

Does this issue also happen for tvOS? Maybe we need to remove the TARGET_DEVICE_PLATFORM_NAME check all together?

Haven't tried with tvOS, but logically it makes sense to apply to all platforms? I can add that as a part of this change if you'd like.

@brentleyjones
Copy link
Contributor

Yes please. Also you need to sign-off on your git commit to pass the DCO check.

@sebastianv1 sebastianv1 force-pushed the sebastianv1/fix-plist-touch branch 2 times, most recently from bc08a7f to 4365b21 Compare March 7, 2024 21:01
@brentleyjones brentleyjones changed the title Include simulator in TARGET_DEVICE_PLATFORM_NAME matching Remove TARGET_DEVICE_PLATFORM_NAME matching from Info.plist touching Mar 7, 2024
It appears that the `TARGET_DEVICE_PLATFORM_NAME` check didn't account for `iphonesimulator` platform name which was causing unit tests running on a test host to not reflect changes made in source code.

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@brentleyjones brentleyjones enabled auto-merge (squash) March 7, 2024 22:25
@brentleyjones brentleyjones merged commit d44b4ed into MobileNativeFoundation:main Mar 8, 2024
14 checks passed
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.

None yet

3 participants