[PATCH] D15801: Improve the documentation on committing code reviewed on Phabricator to trunk.

[Manuel Klimek via llvm-commits]

klimek added inline comments.

================
Comment at: docs/Phabricator.rst:130-136
@@ -129,3 +129,9 @@
 
-Arcanist can manage the commit transparently. It will retrieve the description,
-reviewers, the ``Differential Revision``, etc from the review and commit it to the repository.
+Once a patch has been reviewed and approved on Phabricator it can then be
+committed to trunk.  There are multiple workflows to achieve this. Whichever
+method you follow you must ensure that your commit message ends with the line:
+
+::
+
+  Differential Revision: <URL>
+
----------------
delcypher wrote:
> klimek wrote:
> > I would phrase that less strongly. Remember that email is still the only system of record, so there's no requirement to make phab auto-close issues.
> > 
> > I'd probably rephrase to something like:
> > Note that you can get phabricator to automatically close the review when it finds the following line in the commit message:
> > ...
> > I would phrase that less strongly. Remember that email is still the only system of record, so there's no requirement to make phab auto-close issues.
> 
> I don't really agree with that. Although there is no requirement to use Phabricator for reviews if the decision has been made to use it then I think we should be documenting the way Phabricator should be used that is most helpful to LLVM:
> 
> * Having closed reviews makes searching for un-committed patches easier
> * Having a link in a commit message to the Phabricator review is useful when looking back at LLVM's history
> 
> I'm happy to weaken the language a little (e.g. turning **must** into **should**) if that helps.
How about saying "... consider to ..."?

Phab is currently really just there to make it easier to do email based code reviews. If you want to propose a change to that policy, feel free to propose it on the llvm-dev/cfe-dev mailing lists.


http://reviews.llvm.org/D15801