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 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 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
