-
Notifications
You must be signed in to change notification settings - Fork 38
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
Monorepo #2
Comments
Actually I thought about the same after reading #1. Sounds like a good approach. |
One thing we have to be aware of: Carthage only builds the first framework-project it finds in the repository (source). I don't know if there is a way to circumvent this or even this even outdated, but we should do some investigation before making a decision about going this way. |
Good point. According to Carthage/Carthage#210 (comment) and Carthage/Carthage#213 (comment), we can still have individual framework targets for each API, but they have to live in the same .xcodeproj. (We can use schemes to differentiate between platforms, as we do today.) Definitely something to double-check first. |
When merging the projects into one .xcodeproj it may be beneficial to move the build configuration in xcconfig files and use something like https://github.com/jensmeder/phoenx (disclaimer: developed by a former colleague, using it at COBI) to generate the project files. This would have the benefit that all targets share the same build configuration (more reliable, easier to update) and that they could be easily separated if needed. A side effect is that merge conflicts in the project file could be easily resolved by regenerating the project. Another thing to keep in mind: the build times of PRs will increase quite a lot because the CI always has to build all projects. |
With the Mapbox iOS and macOS SDKs, which are currently part of a monorepo, we went with checking in the .xcodeproj files rather than generating them (mapbox/mapbox-gl-native#4641). However, we used .xcconfig files for the build settings needed to link the targets to another project that was generated by CMake. .xcconfig files are designed to work with checked-in .xcodeproj files. For this project, once we merge the targets into one .xcodeproj, the targets can share build settings pretty easily with or without an .xcconfig. (There’s a trick to writing the .xcconfig files: copy the bolded lines in the Build Settings tab, hit Delete, and paste them into the .xcconfig.)
Hehe, I’m used to half-hour build times in the mapbox-gl-native monorepo, so that didn’t even cross my mind. 😄 We can configure Bitrise triggers so that, for example, branches ending in |
Which is necessary when Carthage should be supported
We generate the project on the fly when checking out. The project then just references the checked-in .xcconfig files. However, this is not possible when having a lib as mentioned above.
true
Yeah, used this one as well when extracting the .xcconfig out of our original project.
Yeah. Could be documented in a |
This repo is going to be telemetry specific so closing this now. We can copy it over to the actual swift core repo when that is created. |
As an alternative to the monolithic framework described in passing in #1, we could keep all the Mapbox Swift frameworks in a monorepo (this repository) and in the same Xcode workspace but still produce multiple frameworks. Installing this repository via Carthage would end up building all the frameworks, but a developer could still link against only the ones they want. Meanwhile, for CocoaPods users, we could create subspecs for each of the frameworks. We could replace the multitude of Bitrise build bots with a single bot for each platform that builds the entire workspace.
Procedurally, we’d rename this repository to mapbox-sdk-cocoa (must think of the AppleScript coders!), graft the history of each Mapbox*.swift repository onto this repository’s history, and also move over any outstanding issues.
(I would advise against the monorepo producing any non-Mach-O frameworks or combining it with the mapbox-java or mapbox-gl-native repository, to avoid the mess in mapbox/mapbox-gl-native#1623.)
/cc @mapbox/ios @Eldorado234 @bsudekum @tmcw
The text was updated successfully, but these errors were encountered: