Date: Wed, 16 Jul 2014 18:38:01 +0200 From: Baptiste Daroussin <bapt@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, koobs@freebsd.org Subject: Re: Phabric IDs / URLs in commits Message-ID: <20140716163801.GQ48710@ivaldir.etoilebsd.net> In-Reply-To: <1741877.WlnOog9LaI@ralph.baldwin.cx> References: <201407111616.s6BGGQFW060195@svn.freebsd.org> <201407151112.51824.jhb@freebsd.org> <53C5B8FD.7070809@FreeBSD.org> <1741877.WlnOog9LaI@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
--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/<number>; 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: <ID-including-D-prefix> 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/<ID>' ala 'review.freebsd.org/<ID>' 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140716163801.GQ48710>