[llvm-dev] [cfe-dev] Phabricator -> GitHub PRs?

Renato Golin via llvm-dev llvm-dev at lists.llvm.org
Tue Jan 28 08:55:37 PST 2020


On Tue, 28 Jan 2020 at 16:41, James Y Knight <jyknight at google.com> wrote:
> I would really not want to review a Pull Request that both has multiple "real" commits, as well as unsquashed "fixup" commits in it. That makes it quite difficult to see the final state of things for any of the commits. Since Github lets you see the diff for the entire PR, if the entire branch is intended to be squashed before commit, it seems potentially-OK (if ugly) for it to temporarily have multiple commits during review, rather than force-pushing a new commit.

Indeed, this works better when the number of commits is small and the
nature of the fixups is trivial.

If the change is significant, then having the old commit laying around
is counter-productive and rebase-squashing is the only sane way.

--renato


More information about the llvm-dev mailing list