Differential D4431
[services] Export DatabaseManagers as CMake Project Authored by • jon on Jul 3 2022, 9:11 AM. Tags None Referenced Files
Details This is part of a larger effort to bring more structure to the C++ codebase https://linear.app/comm/issue/ENG-1310/export-nativecpp-projects-as-cmake-projects The intent is to be able to reference the dependencies as packages, instead of just as GLOB'ing over certain directories; which is a major anti-pattern in CMake as it create a bunch of footguns such as finding multiple files with main() defined, or picking up unrelated code in generated folders. Export DatabaseManagers as a CMake Project nix develop cd native/cpp/CommonCpp/DatabaseManagers rm -rf build && mkdir build && cd build && cmake .. & make -j
Diff Detail
Event TimelineComment Actions Resigning since I'm usually not a good person to set as a first-pass reviewer. Exceptions here Comment Actions
At a high level this diff looks good, but can you update the Test Plan to specify what later diff tests this?
Comment Actions
Can you specify what diff this is, don't want to accept if I can't test the changes at least transitively Comment Actions
Still in the middle of reviving this but filtering out the header changes so the bulk CMake logic can be added. I'll "Request Review" when this is in a better state Comment Actions
Generally when this happens you can submit the "Changes Planned" action so the diff is removed from reviewers' queues. Then, once you've made your changes and are ready to hit "Request Review" it'll pop back up in their queues, but while you're figuring stuff out it won't. Comment Actions
Thanks, the icons are both big and red, so I just assumed "plan changes" was the same as "needs revision". My bad Comment Actions
Could you clarify/link to that diff so I can test it? Would be great to get this landed Comment Actions
Essentially all of native/cpp/CommonCpp is all very tightly coupled, so it's hard to do any single build in isolation. In a later diff I do target_link_libraries to the correct targets to make them actually build. For now I just do the target_include_directories anti-pattern so they work individually. Then I'll fix them later | ||||||||||||||||||||||||||||||||||||||||