alamb edited a comment on pull request #8598: URL: https://github.com/apache/arrow/pull/8598#issuecomment-723114191
So my feedback here is that it is not clear to me what this PR is trying to accomplish (aka answer the question of *why* make the changes in this PR) and thus it is not clear how to review / evaluate it. If the PR's aim is to add support for endianness, I would expect some demonstration that the new code can do something that the old code can't (aka tests) If the PR's aim is to fix bug, I would expect some explanation / demonstration / of something that fails without the changes in the PR and passes with changes in the PR. The bugs this PR's changes fixes are probably obvious to you, but sadly they are not to me :( If the PR's aim is to make the code easier to understand, I would expect some description of why the new code is easier to understand than the old (which will be a subjective judgement, for sure). Since this PR seems to have elements of all three goals, but is light on the explanation, I am struggling to evaluate it concisely ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
