vyasr commented on code in PR #406:
URL: https://github.com/apache/arrow-nanoarrow/pull/406#discussion_r1532402335
##########
CMakeLists.txt:
##########
@@ -154,13 +154,48 @@ else()
endif()
endif()
- install(TARGETS nanoarrow DESTINATION lib)
+ install(TARGETS nanoarrow
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
+ install(TARGETS coverage_config
+ DESTINATION lib
+ EXPORT nanoarrow-exports)
Review Comment:
I agree, this would be the right solution. I didn't do it initially because
1) I was worried about breaking existing users and 2) I didn't know how far the
scope of this PR should go. In hindsight, point 1 is moot because this package
wasn't previously importable anyway, so even with adding the namespace existing
consumers would still see the un-namespaced targets. For point 2, IMO best
practice would be to update all the examples/code within the repo to use the
namespaced target consistently whether they're getting nanoarrow via
FetchContent or `find_package`. However, I think that's probably out of scope
for this PR and can be done in some future work. Would you agree?
--
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
