[PATCH] Add a note about using "Differential Revision:" in commit messages

Manuel Klimek klimek at google.com
Tue Feb 4 01:47:07 PST 2014


On Mon, Feb 3, 2014 at 10:20 PM, Mark Seaborn <mseaborn at chromium.org> wrote:

> Hi chandlerc, klimek,
>
> Add a note about using "Differential Revision:" in commit messages
>
> I noticed this convention from the commit logs.  It seems like it
> would be useful to document it, to encourage other committers to link
> back to code reviews in their commits.
>
>
> http://llvm-reviews.chandlerc.com/D2678
>
> Files:
>   docs/Phabricator.rst
>
> Index: docs/Phabricator.rst
> ===================================================================
> --- docs/Phabricator.rst
> +++ docs/Phabricator.rst
> @@ -94,6 +94,22 @@
>  the web interface. Thus, please type LGTM into the comment box to accept
>  a change from Phabricator.
>
> +Committing a change
> +-------------------
> +
> +When committing an LLVM change, the commit message should end with the
> line:
>

I think this is worded way too strongly - phab is by no way mandatory.
Perhaps:
When committing an LLVM change that has been reviewed via phabricator,
submit messages may end with the line:


> +
> +::
> +
> +  Differential Revision: <URL>
> +
> +where ``<URL>`` is the URL for the code review, starting with
> +``http://llvm-reviews.chandlerc.com/``.
> +
> +This allows people reading the version history to see the review for
> +context.  This also allows Phabricator to detect the commit, close the
> +review, and add a link from the review to the commit.
>

It might be a good idea to add that using arcanist to submit the patches
will automatically add those comments?

I'm also not sure whether people who are less in favor of phabricator (I
know there are quite a few) are actually fine with that. I'd like to hear
some more supportive feedback before encouraging this...

Thanks!
/Manuel


> +
>  Status
>  ------
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140204/89d17a34/attachment.html>


More information about the llvm-commits mailing list