hsutter commented on PR #51270: URL: https://github.com/apache/arrow/pull/51270#issuecomment-5642162671
I took more time to look at the three review comments, and IMO the PR is actually correct about those three proposed changes. I've tried to reply with some rationale why (note all these replies are my manual analysis and writing, NOT Claude; I just happen to still agree with the changes that my Claude skill generated). In the meantime, let me reopen this PR -- I closed it to defer to the push-back that the PR wasn't ready, but now that I've reviewed it myself and still agree, I think there is still value in the PR. However, to repeat a caveat that I wrote above: I have no idea whether these signatures I'm proposing to change might be exported/API functions, e.g., for use in cross-language APIs, that must be pass by value and can't tolerate pass by `const&`. If that is a constraint for any of the ~400 proposed changes please tell me, and if so please let me know how can I tell which types/functions can't tolerate such a parameter passing change so I can exclude them and make this PR better. Thanks again for your time and interest! -- 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]
