Date: Wed, 16 Jul 2014 11:41:34 -0400 From: John Baldwin <jhb@freebsd.org> To: koobs@freebsd.org Cc: svn-src-head@freebsd.org, Baptiste Daroussin <bapt@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: Phabric IDs / URLs in commits Message-ID: <1741877.WlnOog9LaI@ralph.baldwin.cx> In-Reply-To: <53C5B8FD.7070809@FreeBSD.org> References: <201407111616.s6BGGQFW060195@svn.freebsd.org> <201407151112.51824.jhb@freebsd.org> <53C5B8FD.7070809@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, July 16, 2014 09:27:57 AM Kubilay Kocak wrote: > 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. Note that we could choose a "canonical" URL similar to how we have done for 'bugs.freebsd.org/<ID>' ala 'review.freebsd.org/<ID>' or the like. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1741877.WlnOog9LaI>