From owner-svn-src-all@FreeBSD.ORG Wed Jul 16 16:31:16 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0858AC27; Wed, 16 Jul 2014 16:31:16 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D02DE295B; Wed, 16 Jul 2014 16:31:15 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-70-85-31.nwrknj.fios.verizon.net [173.70.85.31]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3E19BB968; Wed, 16 Jul 2014 12:31:12 -0400 (EDT) From: John Baldwin To: koobs@freebsd.org Subject: Re: Phabric IDs / URLs in commits Date: Wed, 16 Jul 2014 11:41:34 -0400 Message-ID: <1741877.WlnOog9LaI@ralph.baldwin.cx> User-Agent: KMail/4.10.5 (FreeBSD/10.0-STABLE; KDE/4.10.5; amd64; ; ) In-Reply-To: <53C5B8FD.7070809@FreeBSD.org> References: <201407111616.s6BGGQFW060195@svn.freebsd.org> <201407151112.51824.jhb@freebsd.org> <53C5B8FD.7070809@FreeBSD.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 16 Jul 2014 12:31:12 -0400 (EDT) Cc: svn-src-head@freebsd.org, Baptiste Daroussin , src-committers@freebsd.org, svn-src-all@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jul 2014 16:31:16 -0000 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/ 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: 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/' ala 'review.freebsd.org/' or the like. -- John Baldwin