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

Renato Golin via cfe-dev cfe-dev at lists.llvm.org
Tue Jan 28 08:31:01 PST 2020


On Tue, 28 Jan 2020 at 16:09, David Greene <dag at cray.com> wrote:
> The question is if everything is approved and the author does a final
> cleanup as alluded to above, does that final cleanup also need to go
> through review?

We don't enforce that now, so I see no reason to start doing it.

Phab reviews, once approved, can have last-minute modifications and
direct commits.

Often reviewers will reply "LGTM with this nit addressed", which means
the author is supposed to fix the code, ammend/squash, and push.

A rebase squash of the fixups is slightly more complex if they need to
apply to different patches in the series, but the author is *expected*
to run a final test on the rebased version *before* pushing.

If not, buildbots will break and patches reverted.

An author that keeps doing that will raise their own bar on future
reviews, where reviewers would start asking them to rebase and apply
before approving and merging.

cheers,
--renato


More information about the cfe-dev mailing list