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

[Renato Golin via llvm-dev]

On Tue, 28 Jan 2020 at 12:28, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> I don't quite follow. Yes, gratuitous useless changes tend to create
> merge and rebase problems and should generally be avoided. Why does it
> make a difference whether they're in multiple commits though?

I think you misunderstood.

Our policy [1] is that independent NFC fixups on existing code should
be separate from new changes.

The point here is that fixups *to the patch* should be squashed into
their original commits *before* merging.

Once the patches are in master, and problems have been found, then
adding a fixup to master is totally fine.

We don't want to rewrite history on master, that'd be a nightmare. If
something is botched, we revert, then reapply.

We also don't want to have fixups to things that haven't landed on
master yet, so cleaning up the patches and series before merging is
highly encouraged.

But during the review, rewriting history of the series itself makes it
hard for any review tool to have meaningful representations.

So, it's better to have separate fixups during review, but we really
should squash them into their related commits in the series before
pushing.

--renato

[1] http://llvm.org/docs/DeveloperPolicy.html