Phabric IDs / URLs in commits

[Kubilay Kocak]

On 16/07/2014 1:12 AM, John Baldwin wrote:
> On Friday, July 11, 2014 1:54:42 pm Baptiste Daroussin wrote:
>> On Fri, Jul 11, 2014 at 12:38:23PM -0400, 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.
>>
>> for bugs we could use http://bugs.FreeBSD.org/<number> that also works and it is
>> short :)
> 
> Ok, so Bryan said ports uses 'Phabric: Dxxx' and I read Baptiste's e-mail as a
> preference for the URL itself (no preference on the prefix though?)  Any other
> thoughts?  I probably lean towards the full URL personally since it requires less
> work (no hacking on svnweb) and works out-of-the-box in more forums (e-mail, etc.)
> 

+100 on CR: <ID-including-D-prefix> without URL's to keep them decoupled
and forever valid in the (probably very likely) case we change
hostnames/urls.

I'm liking phabric so far, but would opt for a more concrete
review.freebsd.org if I had the choice (and when it's ready). This way
our "review" processes and workflows can be extended or modified
orthogonal to the tool in use.