[PATCH] D108314: Revert "[NFC] factor out unrolling decision logic"

Geoffrey Martin-Noble via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 18 12:38:20 PDT 2021


GMNGeoffrey added a comment.

In D108314#2952915 <https://reviews.llvm.org/D108314#2952915>, @lebedev.ri wrote:

> What is the point of creating a review if you are going to commit it immediately?
> Can you not just push the change directly?
> This only creates noise.

Hi Roman. I have heard and read unclear and conflicting guidance on usage of phabricator, llvm-commits, git, and arc for LLVM and how all these relate to patches intended for pre- or post-commit review. To me, having everything on phab makes sense. Additionally, use of `arc` requires creating a diff first, I believe and helps with the fetch+rebase+push flow, which often needs to be repeated if something else was pushed in the meantime. In cases where pre-merge builds would be useful, but review would not, creating a phab diff has the additional advantage of triggering those. Finally, following a consistent workflow helps avoid errors in constructing git commands. For these reasons, I decided that the simplest thing to do was to make my workflow the same in all cases: `arc diff` + `arc land`. I apologize if that has created additional noise for you and I am open to further suggestions, assuming there is something approaching a community consensus on how these things should be done. IMO the LLVM review and development process could use quite a bit of simplification and modernization...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108314/new/

https://reviews.llvm.org/D108314



More information about the llvm-commits mailing list