Copilot commented on code in PR #47042:
URL: https://github.com/apache/arrow/pull/47042#discussion_r2211684533
##########
cpp/src/arrow/flight/test_util.h:
##########
@@ -143,6 +143,9 @@ class ARROW_FLIGHT_EXPORT NumberingStream : public
FlightDataStream {
ARROW_FLIGHT_EXPORT
std::shared_ptr<Schema> ExampleIntSchema();
+ARROW_FLIGHT_EXPORT
Review Comment:
The declaration for `ExampleFloatSchema()` is added but there's no
corresponding implementation visible in the diff. Ensure that the
implementation exists and is properly exported.
##########
cpp/src/arrow/filesystem/filesystem_library.h:
##########
@@ -26,7 +26,9 @@ extern "C" {
// _declspec(dllexport)/[[gnu::visibility("default")]] even when
// this header is #included by a non-arrow source, as in a third
// party filesystem implementation.
-ARROW_FORCE_EXPORT void* arrow_filesystem_get_registry() {
+ARROW_FORCE_EXPORT void* arrow_filesystem_get_registry();
Review Comment:
[nitpick] The function declaration is separated from its definition on line
31. This pattern of declaration followed immediately by definition in a header
is unusual. Consider whether both the declaration and definition need to be in
the header, or if one should be moved to a .cc file.
##########
cpp/src/arrow/extension/tensor_internal.h:
##########
@@ -25,8 +25,7 @@
namespace arrow::internal {
-ARROW_EXPORT
-Status IsPermutationValid(const std::vector<int64_t>& permutation) {
+inline Status IsPermutationValid(const std::vector<int64_t>& permutation) {
Review Comment:
[nitpick] The function `IsPermutationValid` was changed from `ARROW_EXPORT`
to `inline` but remains in a header file. If this function is only used
internally within a single compilation unit, consider moving it to the
corresponding .cc file instead of making it inline in the header.
--
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
