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

[Manuel Klimek]

On Wed, Feb 5, 2014 at 10:49 PM, Mark Seaborn <mseaborn at chromium.org> wrote:

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

There are definitely people who object strongly to phabricator - I've heard
some concerns about the logs, but by now it might be common enough that
there are no strong objections left...

So, LG, if we get push-back we can always revert the docs ...
Thanks!
/Manuel


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