Convert the codebase to Objective-C++ #1206
Conversation
| - (NSDictionary<NSString *,ASPerformanceTestResult *> *)results { | ||
| return _results; | ||
| } | ||
|
|
There was a problem hiding this comment.
Note: C++ was complaining that our ivar is NSMutableDictionary, not NSDictionary.
| [cv reloadData]; | ||
| } completion:nil]); | ||
| } | ||
|
|
There was a problem hiding this comment.
Note: This test is broken now. It involves intentionally causing UICollectionView to throw an exception, and the view now ends up in such a state that when it deallocates, it still thinks there's an update in progress which isn't allowed.
The test doesn't really do much. It just confirms an edge case in UICollectionView's behavior.
|
Hmm the linker is now much stricter! We aren't automagically getting CoreMedia and AVFoundation linked for us (kind of good to have it called out). I added those to the list of optional dependencies alongside MapKit and AssetsLibrary. |
|
The build is failing because Weaver explicitly only imports "Texture/Core", not the default subspec. In order to make the linker happy, this diff takes AVFoundation and CoreMedia parts (ASVideoNode) out of Core but leaves them in the default subspec. Huy is the only one authorized to push new Weaver cocoapods versions. I already cut a release that points Weaver to the full Texture. @nguyenhuy could you pull master on Weaver and run "pod trunk push" please? |
|
To get the build working, I've pointed ASDKgram's import of Weaver explicitly to the 0.1.2 branch until the pod is pushed. |
|
Will do!
…On November 1, 2018 at 5:21:01 PM, Adlai Holler ***@***.***) wrote:
The build is failing because Weaver explicitly only imports
"Texture/Core", not the default subspec. In order to make the linker happy,
this diff takes AVFoundation and CoreMedia parts (ASVideoNode) out of Core
but leaves them in the default subspec.
Huy is the only one authorized to push new Weaver cocoapods versions. I
already cut a release that points Weaver to the full Texture. @nguyenhuy
<https://github.com/nguyenhuy> could you pull master on Weaver and run
"pod trunk push" please?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1206 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAj4YslA7WoEHyfZdz_1ymTYC7I97JIwks5uq4_tgaJpZM4YI5q->
.
|
|
That was fast! Nice! |
|
OK I think the linker change was Xcode 10.1, not the Objective-C++ change. I've reverted those parts for another day. Would love people's thoughts on this – if we all agree, I'd like to get to work on cleaning up some of the detritus caused by interweaving .m and .mm files together. |
|
🎉 Congrats 🚀 Weaver (0.1.2) successfully published |
|
Thanks for doing that so fast. Sorry I wasted your time and reverted – feature detection is really tricky and it got too weird. We'll revisit it in a separate PR. |
Adlai-Holler
force-pushed
the
AHAllCxx
branch
from
dc0e7c3 to
31c1a07
Compare
Adlai-Holler
force-pushed
the
AHAllCxx
branch
from
eb66f41 to
18b4b5c
Compare
Generated by 🚫 Danger |
* Convert the codebase to Objective-C++ throughout. One language is better than two. * Put it back * Fix linker * Point explicitly to updated Weaver to unblock build * Revert "Point explicitly to updated Weaver to unblock build" This reverts commit fdc2529. * Revert "Fix linker" This reverts commit 7be25f9. * Add in the frameworks * no message * Address spec lint warnings * Fix tvos build * Put that back * Address Michael's review * Add comment to kick CI
Now that we've turned off ARC exception safety, and now that Xcode's refactoring engine supports Objective-C++, let's stop mixing languages and standardize on Objective-C++ for all source and test files.
We'll still not leak C++ into our public headers. Sample apps should still be written in plain Objective-C.
This sets the stage for a lot more cleanup and optimization in the near future.