[PATCH] D156445: [BPF] Avoid repeating MI->getOperand(NumDefs) x3
Eduard Zingerman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 2 09:33:04 PDT 2023
eddyz87 added a comment.
In D156445#4554360 <https://reviews.llvm.org/D156445#4554360>, @tamird wrote:
> In D156445#4554232 <https://reviews.llvm.org/D156445#4554232>, @eddyz87 wrote:
>
>> In my opinion, whether or not to repeat `MI->getOperand(NumDefs)` is a matter of personal preference in this case. Current code is ok and this change does not change any related functionality. I suggest to withheld from this change to avoid cluttering commit history.
>
> I seems obvious that readability of code-at-HEAD should be of a higher priority than the readability of the commit history; the extant code will be read many more times than the the commit history for the same piece of code.
I disagree that readability of these 2 lines had improved. I see this commit is an unnecessary code shuffling.
> The description of this change doesn't claim that there is a change in behavior here; is there documentation that requires all changes to have functional changes?
>
>> Also, I don't understand why you landed this change w/o due process.
>
> As I understand it, the process is not prescriptive about reviews being required. I see many changes (particularly in this area) that have gone in without review. In this case, I tried to follow "due process", but received no engagement whatsoever. Is filibustering an accepted practice?
Personally, I wait for reviewers engagements for all my changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156445/new/
https://reviews.llvm.org/D156445
More information about the llvm-commits
mailing list