pitrou commented on code in PR #46161: URL: https://github.com/apache/arrow/pull/46161#discussion_r2055704318
########## cpp/src/arrow/compute/kernels/test_util.h: ########## @@ -0,0 +1,30 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: > I don't think it's great that we are exporting symbols from `test_util_internal.cc` and that we are adding some of the internals to `libarrow_testing.so` It's a bit confusing but these all have slightly different purposes: 1) the `internal` namespace tells that an API is not for public use 2) the `_internal.h` suffix is for private headers that are not installed, so that third-party code can not include them; the APIs in those headers are implicitly internal, even if not in an `internal` namespace 3) we can still need to export internal APIs in our DLLs, because the unit tests may need them and are usually dynamically linked 4) we can still need to put internal APIs in public headers, for example because they are used in public inline functions 5) we unfortunately have APIs that are morally internal, but have not been put in a `internal` namespace or a `_internal.h` header Here we have an internal API that is used by unit tests, so needs to be exported in a DLL. -- 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
