[docs] Phabricator, not the lists is the main entry point for new patches

[lebedev.ri]

Summary

I've seen people trip over the current friendly-but-not-true wording more than once.
I'm reasonably sure that all the new patches generally go to phabricator, not lists.
How about we reword it to be more specific, and prevent misdirections?

[lebedev.ri]

docs/Contributing.rst

58–59

I understand that it says that the review can happen on lists too, but the general public will
not read the second line here about first submitting the patch to Phabricator, and *then* reviewing on lists,
so it is confusing to mention lists here.

[probinson]

Requiring Phabricator raises the barrier to one-off patches from casual contributors, because using Phabricator requires a registration step.
I don't think we should require it until casual users with drive-by patches can contribute easily.

[hfinkel]

In D63488#1548514, @probinson wrote:

Requiring Phabricator raises the barrier to one-off patches from casual contributors, because using Phabricator requires a registration step.
I don't think we should require it until casual users with drive-by patches can contribute easily.

But my impressions is that, in practice, patches sent to the mailing list are not tracked, more difficult to review, and often, don't get looked at. Having a patch sent to the mailing list only to be ignored is a larger barrier to entry than the registration step. We should ask people to follow a procedure that is like to work for them, not one likely to end in frustration.

[lebedev.ri]

In D63488#1548725, @hfinkel wrote:

In D63488#1548514, @probinson wrote:

Requiring Phabricator raises the barrier to one-off patches from casual contributors, because using Phabricator requires a registration step.
I don't think we should require it until casual users with drive-by patches can contribute easily.

But my impressions is that, in practice, patches sent to the mailing list are not tracked, more difficult to review, and often, don't get looked at. Having a patch sent to the mailing list only to be ignored is a larger barrier to entry than the registration step. We should ask people to follow a procedure that is like to work for them, not one likely to end in frustration.

That's quite precisely my point.
It's friendlier to write the *actual* steps, even if they are more complicated than the
friendly "just submit the patch somewhere, it's their job to track every single entry point"
which is ultimately not true and results in silently ignored patches.

[probinson]

I see the point; certainly when someone emails a patch, the first response is almost always "can you put this on Phab."

There was talk at some point about connecting Phab with github authentication, somehow? If people can use an existing account then the you-need-to-register-first objection goes away.

[probinson]

In any event, this is a project policy change and should have an RFC on llvm-dev, not be proposed in a patch.

[davide]

In D63488#1548991, @probinson wrote:

I see the point; certainly when someone emails a patch, the first response is almost always "can you put this on Phab."

There was talk at some point about connecting Phab with github authentication, somehow? If people can use an existing account then the you-need-to-register-first objection goes away.

I log on Phabricator using GitHub every day, but my account was activated a while ago, so I'm not sure if that was deactivated in the meanwhile.

[lebedev.ri]

So, i did submit the RFC in https://lists.llvm.org/pipermail/llvm-dev/2019-June/133162.html
There were 6 replies from 5 people, summary:

• @jhenderson +1
• @probinson +0.0f
• @rnk +1
• @hfinkel +1
• @reames +1

So no one opposed this, everyone liked it.
The point about lists being subscribed, and being ready to reply to
mail-only comments i did not change, i believe that is all already stated.

Now, anyone care to stamp this? :)

[reames]

I will be willing to LGTM this, but first, a couple of suggested changes.

docs/DeveloperPolicy.rst

101

I'm fine with the change of tone, but deleting the specific mailing list advise about how to do so is probably a poor idea. It'd be good to shift those two paragraphs past the one about confidential information, and maybe lead in with something along the lines of:

It is also acceptable to submit patches directly to the llvm-commit mailing list. We discourage this for new users though as it less likely your patch will be seen by motivated reviewers as many who watch for contributions from new contributors rely on phabricator for filtering.

docs/Phabricator.rst

16

Addition: Such comments are almost always mirrored into phabricator automatically, but sometimes the scripting isn't quite perfect. Please check manually before committing.

docs/Contributing.rst

 =====================
 Once you have a patch ready, it is time to submit it. The patch should:
 
 * include a small unit test
 * conform to the :doc:`CodingStandards`. You can use the `clang-format-diff.py`_ or `git-clang-format`_ tools to automatically format your patch properly.
 * not contain any unrelated changes
 * be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.
 
 To get a patch accepted, it has to be reviewed by the LLVM community. This can
-be done using `LLVM's Phabricator`_ or the llvm-commits mailing list.
+be done using `LLVM's Phabricator`_.
 Please  follow :ref:`Phabricator#requesting-a-review-via-the-web-interface <phabricator-request-review-web>`
 to request a review using Phabricator.
 
 To make sure the right people see your patch, please select suitable reviewers
 and add them to your patch when requesting a review. Suitable reviewers are the
 code owner (see CODE_OWNERS.txt) and other people doing work in the area your
 patch touches. If you are using Phabricator, add them to the `Reviewers` field
 when creating a review and if you are using `llvm-commits`, add them to the CC of

docs/DeveloperPolicy.rst

 #. Similarly, patches should be submitted soon after they are generated.  Old
    patches may not apply correctly if the underlying code changes between the
    time the patch was created and the time it is applied.
 
 #. Patches should be made with ``git format-patch``, or similar. If you use a
    different tool, make sure it uses the ``diff -u`` format and that it
    doesn't contain clutter which makes it hard to read.
 
-Once your patch is ready, submit it by emailing it to the appropriate project's
-commit mailing list (or commit it directly if applicable). Alternatively, some
-patches get sent to the project's development list or component of the LLVM bug
-tracker, but the commit list is the primary place for reviews and should
-generally be preferred.
-
-When sending a patch to a mailing list, it is a good idea to send it as an
-*attachment* to the message, not embedded into the text of the message.  This
-ensures that your mailer will not mangle the patch when it sends it (e.g. by
-making whitespace changes or by wrapping lines).
-
-*For Thunderbird users:* Before submitting a patch, please open *Preferences >
-Advanced > General > Config Editor*, find the key
-``mail.content_disposition_type``, and set its value to ``1``. Without this
-setting, Thunderbird sends your attachment using ``Content-Disposition: inline``
-rather than ``Content-Disposition: attachment``. Apple Mail gamely displays such
-a file inline, making it difficult to work with for reviewers using that
-program.
+Once your patch is ready, please  follow
+:ref:`Phabricator#requesting-a-review-via-the-web-interface <phabricator-request-review-web>`
+to request a review using Phabricator (or commit it directly if applicable).
+Alternatively, some patches get sent to the project's development list or
+component of the LLVM bug tracker, but the Phabricator is the primary place for
+reviews and should generally be preferred.
 
 When submitting patches, please do not add confidentiality or non-disclosure
 notices to the patches themselves.  These notices conflict with the LLVM
 licensing terms and may result in your contribution being excluded.
 
 .. _code review:
 
 Code Reviews

docs/Phabricator.rst

 .. _phabricator-reviews:
 
 =============================
 Code Reviews with Phabricator
 =============================
 
 .. contents::
   :local:
 
-If you prefer to use a web user interface for code reviews, you can now submit
-your patches for Clang and LLVM at `LLVM's Phabricator`_ instance.
+You can submit your patches for Clang and LLVM at `LLVM's Phabricator`_
+instance.
 
 While Phabricator is a useful tool for some, the relevant -commits mailing list
 is the system of record for all LLVM code review. The mailing list should be
 added as a subscriber on all reviews, and Phabricator users should be prepared
 to respond to free-form comments in mail sent to the commits list.
 
 Sign up
 -------
 
 To get started with Phabricator, navigate to `https://reviews.llvm.org`_ and
 click the power icon in the top right. You can register with a GitHub account,
 a Google account, or you can create your own profile.