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

Xcode10 build fix when using carthage #908

Closed
wants to merge 2 commits into from
Closed

Xcode10 build fix when using carthage #908

wants to merge 2 commits into from

Conversation

Nonnus
Copy link
Contributor

@Nonnus Nonnus commented Aug 14, 2018

Thanks for proposing a pull request.

To help us review the request, please complete the following:

  • sign contributor license agreement
  • submit against our dev branch, not master.
  • describe the change (for example, what happens before the change, and after the change)

This pr fixes compiling with latest xcode 10 using carthage by implementing the fixes described in facebookarchive/facebook-swift-sdk#246

@vitorhugomagalhaes
Copy link

Any update on this please ?

@Nonnus
Copy link
Contributor Author

Nonnus commented Aug 22, 2018

i created the pr, now was waiting for someone to review it...

@vitorhugomagalhaes
Copy link

I'll use your fork to move forward. Thanks

@Nonnus
Copy link
Contributor Author

Nonnus commented Aug 22, 2018

@vitorhugomagalhaes keep in mind i also had to patch Bolts (BoltsFramework/Bolts-ObjC#323)

in the end to be able to keep working on xcode 10 i ended up making 3 forks and just set my cartfile to point to github "hellofresh/Facebook-SDK-Swift", feel free to use it as well ;)

@vitorhugomagalhaes
Copy link

Great. Thanks for sharing.

@codytwinton
Copy link
Contributor

@vitorhugomagalhaes @Nonnus - we had an issue with the latest iOS SDK release where PRs were accidentally reverted on release. I'm doing some internal work to resolve this issue and I don't want to merge this PR until I can verify that it won't be reverted.

#880 (comment)

Copy link
Contributor

@codytwinton codytwinton left a comment

Choose a reason for hiding this comment

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

This all looks great, but we had an issue with the latest iOS SDK release where PRs were accidentally reverted on release. I'm doing some internal work to resolve this issue and I don't want to merge this PR until I can verify that it won't be reverted.

@vitorhugomagalhaes
Copy link

Any update here ?

@codytwinton codytwinton changed the base branch from dev to master September 12, 2018 19:50
@codytwinton
Copy link
Contributor

Thank you for this PR! I'll import these changes internally and get it shipped asap.

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.

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

@Nonnus
Copy link
Contributor Author

Nonnus commented Sep 12, 2018

@codytwinton my pleasure, happy I could help ;)

@vitorhugomagalhaes
Copy link

@Nonnus Did you confirmed that the dynamic Bolts framework was being generated on your PR. The hello fresh clone seems to be missing that and then the app will not be able to load it at runtime as required by the other FBSDK frameworks which do rely on the dynamic version of Bolts.

@codytwinton
Copy link
Contributor

Still waiting on the PR in Bolts to be merged.

@sesang06 sesang06 mentioned this pull request Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants