Phabric IDs / URLs in commits

[Bryan Drewery]

On 7/11/2014 11:47 AM, Bryan Drewery wrote:
> On 7/11/2014 11:38 AM, John Baldwin wrote:
>> On Friday, July 11, 2014 12:16:26 pm John Baldwin wrote:
>>> Author: jhb
>>> Date: Fri Jul 11 16:16:26 2014
>>> New Revision: 268531
>>> URL: http://svnweb.freebsd.org/changeset/base/268531
>>>
>>> Log:
>>>   Fix some edge cases with rewinddir():
>>>   - In the unionfs case, opendir() and fdopendir() read the directory's full
>>>     contents and cache it.  This cache is not refreshed when rewinddir() is
>>>     called, so rewinddir() will not notice updates to a directory.  Fix this
>>>     by splitting the code to fetch a directory's contents out of
>>>     __opendir_common() into a new _filldir() function and call this from
>>>     rewinddir() when operating on a unionfs directory.
>>>   - If rewinddir() is called on a directory opened with fdopendir() before
>>>     any directory entries are fetched, rewinddir() will not adjust the seek
>>>     location of the backing file descriptor.  If the file descriptor passed
>>>     to fdopendir() had a non-zero offset, the rewinddir() will not rewind to
>>>     the beginning.  Fix this by always seeking back to 0 in rewinddir().
>>>     This means the dd_rewind hack can also be removed.
>>>   
>>>   While here, add missing locking to rewinddir().
>>>   
>>>   CR:   	    	https://phabric.freebsd.org/D312
>>>   Reviewed by:	jilles
>>>   MFC after:	1 week
>>
>> Just picking my own commit here as a sample case.
>>
>> I think we should be annotating commits with phabricator code reviews in some 
>> way when a change has gone through that review.  It is very useful to get back
>> to the review details from the commit log message in svnweb, etc.
>>
>> I can see a number of different ways to do this, but I do think it would be
>> nice to pick a consistent way to do it.
>>
>> Things to consider:
>>
>> 1) The tag ("CR:" is what I used above).  I don't care, just pick one.  I
>>    chose CR since Warner used it previously.  Whatever we decide, we should
>>    add it to the template.
>>
>> 2) ID vs full URL.  For PRs we just list the bug ID and not the full URL
>>    (same for Coverity).  I would be fine with that so long as someone hacks
>>    up svnweb to convert the IDs into links (the way it handles PR bug
>>    numbers).  OTOH, if you use the full URL you get that for free in svnweb,
>>    and you also get it in mail clients, etc.  It helps that the URL isn't but
>>    so long.
>>
>> This is more of a pie-in-the-sky, but it would be _really_ nice if arcanist 
>> were hacked up to support our local commit template and would auto populate 
>> the 'Reviewed by' and 'CR' (or whatever it ends up being called) fields so one 
>> could use 'arc commit'.
>>
>> So what do folks prefer for 1) and 2)?
>>
> 
> FYI Ports has been using the convention: "Phabric\tDXXX"
> 

'Phabric:\tDXXX' with : of course.

-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 553 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20140711/1ecc44db/attachment.sig>