paleolimbot opened a new issue, #88:
URL: https://github.com/apache/arrow-nanoarrow/issues/88

   The initial proof-of-concept version of the IPC extension (#61) added a 
minimal CMake configuration to get started; however, it is almost certainly not 
an ideal build configuration for the real world. There are at least three 
high-level changes that might be useful
   
   First, the extension follows a pattern where "nanoarrow_ipc.h" does not 
`#include "nanoarrow.h"`. This follows our own documentation's advice around 
how libraries should use nanoarrow...nanoarrow should generally be considered 
an implementation detail, not the basis for an extension library. However, for 
the extensions we define internally like the IPC extension, it's very likely 
that the caller is going to want to use nanoarrow and thus following our own 
advice would necessitate two copies of nanoarrow for most users. I think that 
extensions should be allowed to `#include "nanoarrow.h"` for this reason and to 
reduce duplication since many basic library concepts overlap (e.g., 
`ArrowError`, `ArrowErrorCode`, `ArrowBufferView`).
   
   Second, the cmake contains some `FetchContent` logic to build and link the 
flatcc flatbuffers implementation. There should almost certainly be a "link to 
shared" option (maybe via `find_package()`) and/or a vendored option since 
flatcc is a bit of an obscure dependency.
   
   Third, we should figure out how to bundle the extension so that it can be 
added to the `dist/` folder like nanoarrow.h and nanoarrow.c. To allow for some 
build flexibility with flatcc, I imagine this should be a few files: 
nanoarrow_ipc.h, nanoarrow_ipc_generated.h, and nanoarrow_ipc_flatcc.c.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to