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 > > >>>> > > >>>> 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
Btw the template is deeply related to phabricator, making a freebsd specific template will be hard. Over to volunteer :) regards, Bapt
pgpK6JQuQGz0X.pgp
Description: PGP signature