Date: Wed, 16 Jul 2014 09:27:57 +1000 From: Kubilay Kocak <koobs@FreeBSD.org> To: John Baldwin <jhb@freebsd.org>, Baptiste Daroussin <bapt@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: Phabric IDs / URLs in commits Message-ID: <53C5B8FD.7070809@FreeBSD.org> In-Reply-To: <201407151112.51824.jhb@freebsd.org> References: <201407111616.s6BGGQFW060195@svn.freebsd.org> <201407111238.23391.jhb@freebsd.org> <20140711175442.GJ93051@ivaldir.etoilebsd.net> <201407151112.51824.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53C5B8FD.7070809>