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

[Fabric]: Remove cxx from iOS project #25392

Closed
wants to merge 2 commits into from
Closed

[Fabric]: Remove cxx from iOS project #25392

wants to merge 2 commits into from

Conversation

zhongwuzw
Copy link
Contributor

@zhongwuzw zhongwuzw commented Jun 25, 2019 •
edited
Loading

Summary

Remove cxx from iOS project, because we have the corresponding implementation in iOS. Otherwise, it would break the logical handling of iOS part.

Changelog

[iOS] [Fixed] - Remove cxx from iOS project

Test Plan

Fabric enabled Tester can work like loading image.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 25, 2019
@zhongwuzw zhongwuzw requested a review from shergin June 25, 2019 11:02
@facebook facebook deleted a comment from pull-bot Jun 25, 2019
@react-native-bot react-native-bot added Platform: iOS iOS applications. Bug labels Jun 25, 2019
@shergin
Copy link
Contributor

shergin commented Jun 26, 2019

Those path fragments in exclude_files (sic!) sections.

@shergin shergin closed this Jun 26, 2019
@zhongwuzw
Copy link
Contributor Author

@shergin ? emm, I don't get what your point.

@shergin
Copy link
Contributor

shergin commented Sep 10, 2019

Oops, seems I was very wrong about that. The changes actually make a lot of sense: we don't need cxx and android folders to compile those targets on iOS.

@shergin shergin reopened this Sep 10, 2019
facebook-github-bot
facebook-github-bot reviewed Sep 10, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @zhongwuzw in 46d6e2a.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Sep 10, 2019
@zhongwuzw zhongwuzw deleted the fabric_remove_cxx branch September 12, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@shergin shergin Awaiting requested review from shergin

Assignees
No one assigned
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zhongwuzw @shergin @react-native-bot @facebook-github-bot