[RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches

The current documentation talks about both the Phabricator review, and review
as mail replies on -commits lists. It also talks about submitting
patches to lists,
with the subtext that it may be friendlier for outsiders.

It is true that Phabricator has some entry threshold, larger than
github, or maillists,
so the attempt is not unwarranted. But from what i can tell, 99.9% patches go
via Phabricator. There is a large chance that such a mail-only patch
will simply be
overlooked, ignored, or the very first reply will be "Please post the patch to
Phabricator".

Both of these cases i would call counter-welcoming.
I don't think that is what we want?

I propose to fix the docs to specify that all new patches should go
via Phabricator, not lists:
https://reviews.llvm.org/D63488

Roman.

I don’t think I’ve ever looked at llvm-commits for patches that need reviewing except for when I was looking to actively get involved in reviewing an area I wasn’t already subscribed to by default. Certainly, if somebody posted a patch on llvm-commits to one of the binutils like llvm-readelf or llvm-objcopy, I’d be interested in it, but probably wouldn’t see it nowadays, as I have too great a workload to filter through the mailing lists, whereas I can setup Herald rules to get myself auto-subscribed/added as a reviewer to relevant patches on Phabricator.

Registering isn’t a complicated process, compared to developing a patch in most cases anyway.

That’s a long way of saying +1.

(Resend with the list re-added)

I believe the history is that when Phab was initially introduced, we wrote the documentation this way to make things easy for reviewers who didn’t want to change their workflow. But, I agree with your observations. The majority of code review seems to happen on Phabricator, and the best way to get traction on a new patch is to upload it to Phab and add a few reviewers by name. Regardless of what workflow reviewers would prefer, I think the documentation should recommend Phabricator over email to first time contributors, since, in my experience, it gets better results.

I believe the history is that when Phab was initially introduced, we wrote the documentation this way to make things easy for reviewers who didn't want to change their workflow. But, I agree with your observations. The majority of code review seems to happen on Phabricator, and the best way to get traction on a new patch is to upload it to Phab and add a few reviewers by name. Regardless of what workflow reviewers would prefer, I think the documentation should recommend Phabricator over email to first time contributors, since, in my experience, it gets better results.

+1

-Hal

+1

With one important note which is that the documentation should not that authors are expected to watch llvm-commit for responses, since not all of them make it to phabricator. And definitely emphasize the need to add llvm-commits explicitly to the review!

Philip

And definitely emphasize the need to add llvm-commits explicitly to the review!

If I’m not mistaken, there’s a Herald rule that now does this. Also, make sure llvm-commits is added as a subscriber, not a reviewer!

+1 for clarity as it is friendlier for outsiders