From owner-svn-src-head@FreeBSD.ORG Wed Jul 16 16:38:08 2014 Return-Path: Delivered-To: svn-src-head@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 98258E37; Wed, 16 Jul 2014 16:38:08 +0000 (UTC) Received: from mail-wi0-x22f.google.com (mail-wi0-x22f.google.com [IPv6:2a00:1450:400c:c05::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 90E0C2A2C; Wed, 16 Jul 2014 16:38:07 +0000 (UTC) Received: by mail-wi0-f175.google.com with SMTP id ho1so6517678wib.14 for ; Wed, 16 Jul 2014 09:38:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=QtjMPkxzHjTjUTbzQFIjG5p25ktGFa/skdhStMiJ58c=; b=EGkONiH4NGXynkQlsbxz26z6HbzM6PcIfayM1nVz9xg1LH5M5Sgvh9aouO/Wjog3Pc Vf1zllgOhVO4I7guVIaaH3qRKEr5UFAmCQDzVGBOyxgiO2Y5m0YCUa6bEIhbO37aAjpi Nrhhd6F610CBEFXv1VLw1y+d8oPBQPdDPQDk+O7eZj91IXBXvbd6fy/OCUzoO6dh6hXU 8jERFCu/YqZun9Z/ri+cQFRtGcPWqpmpG0+cuxC8FcYwT6kXk0wpRCI78a8Xwgnz4Koo bIjAxigu1cMJS1iCW0i7Ipq3CL8RW4eqfnfFkiar29rzW1RRqShAaKEfV/ZfEWl6BV8n liKA== X-Received: by 10.194.9.198 with SMTP id c6mr5679058wjb.131.1405528684588; Wed, 16 Jul 2014 09:38:04 -0700 (PDT) Received: from ivaldir.etoilebsd.net ([2001:41d0:8:db4c::1]) by mx.google.com with ESMTPSA id da9sm10842745wib.5.2014.07.16.09.38.03 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 16 Jul 2014 09:38:03 -0700 (PDT) Sender: Baptiste Daroussin Date: Wed, 16 Jul 2014 18:38:01 +0200 From: Baptiste Daroussin To: John Baldwin Subject: Re: Phabric IDs / URLs in commits Message-ID: <20140716163801.GQ48710@ivaldir.etoilebsd.net> References: <201407111616.s6BGGQFW060195@svn.freebsd.org> <201407151112.51824.jhb@freebsd.org> <53C5B8FD.7070809@FreeBSD.org> <1741877.WlnOog9LaI@ralph.baldwin.cx> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="19HmC3QOnaNVzKTI" Content-Disposition: inline In-Reply-To: <1741877.WlnOog9LaI@ralph.baldwin.cx> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, koobs@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jul 2014 16:38:08 -0000 --19HmC3QOnaNVzKTI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 16, 2014 at 11:41:34AM -0400, John Baldwin wrote: > 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 > > >>>>=20 > > >>>> Log: > > >>>> Fix some edge cases with rewinddir(): > > >>>> - In the unionfs case, opendir() and fdopendir() read the direct= ory's > > >>>> full > > >>>> =20 > > >>>> 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. > > >>>> =20 > > >>>> - If rewinddir() is called on a directory opened with fdopendir() > > >>>> before > > >>>> =20 > > >>>> any directory entries are fetched, rewinddir() will not adjust= the > > >>>> seek > > >>>> location of the backing file descriptor. If the file descript= or > > >>>> 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. > > >>>> =20 > > >>>> While here, add missing locking to rewinddir(). > > >>>> =20 > > >>>> CR: https://phabric.freebsd.org/D312 > > >>>> Reviewed by: jilles > > >>>> MFC after: 1 week > > >>>=20 > > >>> Just picking my own commit here as a sample case. > > >>>=20 > > >>> I think we should be annotating commits with phabricator code revie= ws in > > >>> some way when a change has gone through that review. It is very us= eful > > >>> to get back to the review details from the commit log message in > > >>> svnweb, etc. > > >>>=20 > > >>> 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. > > >>>=20 > > >>> Things to consider: > > >>>=20 > > >>> 1) The tag ("CR:" is what I used above). I don't care, just pick o= ne.=20 > > >>> I > > >>>=20 > > >>> chose CR since Warner used it previously. Whatever we decide, we > > >>> should > > >>> add it to the template. > > >>>=20 > > >>> 2) ID vs full URL. For PRs we just list the bug ID and not the ful= l URL > > >>>=20 > > >>> (same for Coverity). I would be fine with that so long as someo= ne > > >>> hacks > > >>> up svnweb to convert the IDs into links (the way it handles PR b= ug > > >>> 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. > > >>=20 > > >> for bugs we could use http://bugs.FreeBSD.org/ that also wor= ks > > >> and it is short :) > > >=20 > > > 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 thoug= h?)=20 > > > 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. > >=20 > > 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. >=20 > 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. >=20 > --=20 > John Baldwin Btw the template is deeply related to phabricator, making a freebsd specific template will be hard. Over to volunteer :) regards, Bapt --19HmC3QOnaNVzKTI Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlPGqmkACgkQ8kTtMUmk6ExFlQCeL3dVkYhxHT7bdwYt7jj64Rkw riAAn2+wmOdw6O5trTH4h20v6l8IQ5G3 =hZa7 -----END PGP SIGNATURE----- --19HmC3QOnaNVzKTI--