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. _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"