paleolimbot commented on PR #719: URL: https://github.com/apache/arrow-nanoarrow/pull/719#issuecomment-2730199643
> I think you should change nanoarrow explicitly to nanoarrow_static to be consistent Done! I do like this better. I also added configurable test linkage for better feedback if this breaks for some reason. > then add a nanoarrow::nanoarrow alias to it This is also sort of a breaking change (`target_link_libraries(... nanoarrow::nanoarrow)` vs `target_link_libraries(... nanoarrow)` and I think explicit `nanoarrow_static` is better (possibly at the expense of being slightly confusing to somebody who doesn't know what static linking is, like me threeish years ago 🙂 ). I'm personally OK with the breaking change here but could definitely be convinced otherwise. > For packaging the shared bit could be made a component that also changes which target gets the main alias. I'll leave this one for now since it doesn't affect the interface and because our compile times are very low but it's a great point that the average we-use-nanoarrow-as-a-subproject only needs one of the two. > nit: You should be able to create an INTERFACE target for the sources and use that instead of variables I'll add a note about this one in #726 since it's in the category of nice-to-have CMake improvements that I don't quite understand yet 🙂 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
