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

[Mark Seaborn]

On 4 February 2014 01:47, Manuel Klimek <klimek at google.com> wrote:

> On Mon, Feb 3, 2014 at 10:20 PM, Mark Seaborn <mseaborn at chromium.org>wrote:
>
>> 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:
>

OK, I've changed the wording.


> +
>> +::
>> +
>> +  Differential Revision: <URL>
>> +
>> +where ``<URL>`` is the URL for the code review, starting with
>> +``http://llvm-reviews.chandlerc.com/``<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?
>

OK, done.  I suspected Arcanist might do that, but I haven't tried using it
yet.


> 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...
>

Can there be any objection to linking to the code review from a commit
message?  Chromium changes have similar review links, and it's very useful
for getting more information about a change.

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/b11d0b80/attachment.html>