On Mon, Nov 3, 2008 at 2:36 PM, Reinier Lamers <[EMAIL PROTECTED]> wrote: > Hi David e.a. > > On Monday 03 November 2008 01:05:23 David Roundy wrote: >> On Fri, Oct 31, 2008 at 5:06 PM, Reinier Lamers <[EMAIL PROTECTED]> > wrote: >> > On Wednesday 29 October 2008 03:39:14 David Roundy wrote: >> >> > Sun Oct 26 16:40:09 GMT 2008 [EMAIL PROTECTED] >> >> > * add a get_unrecorded_in_files to check for unrecorded changes in a >> >> > subset of working directory >> >> > >> >> > Sun Oct 26 19:06:12 GMT 2008 [EMAIL PROTECTED] >> >> > * make get_unrecorded_private work with type witnesses again >> >> > >> >> > Sun Oct 26 19:46:36 GMT 2008 [EMAIL PROTECTED] >> >> > * make whatsnew use the lstat-saving functions to scan the working >> >> > copy >> >> >> >> These look very appealing, and have some nice ideas, but are also buggy. >> >> >> >> :( >> > >> > Here's a patch on top of these that removes the bugs you saw. It moves >> > the unsafeness deeper inward: the juggling with Sealed goes from >> > WhatsNew.lhs to Internal.lhs, the concatenation of primitive patches goes >> > from Internal.lhs to Diff.lhs. >> > >> > If you review this patch, please also watch carefully if I get the stuff >> > with the Seal's right in Internal.lhs. It took some restructuring to make >> > the type checker happy. >> >> Could you resend this as a fresh send to darcs-unstable? I'd like a >> bundle I could apply directly. Even better would be an amend-recorded >> bundle, so it'll be easier to review. > > Here's a bundle against the current unstable. It also adds some tests as Jason > suggested over IRC. I even found a new bug with those tests, but it's not in > my lstat-saving code (see issue1196).
Oh, +5 for test cases, especially ones that find bugs outside of the code you're submitting! Rock on :) Would you mind summarizing each patch as per the section "Things we like": http://wiki.darcs.net/DarcsWiki/DeveloperGettingStarted By which I mean, could you provide more summary and explanation of what you changed and why you hope it's correct. This helps the reviewer crowd a bit. We (well, I do at least) appreciate this work. Especially those tests cases. Makes my day :) I haven't had the time to carefully review it. That's mostly because some of your patches do thing that the later patches undo. I have no problems with that, others may feel differently, eg., you may get a request that you unrecord and make a more cohesive patch with less back and forth. That said, things I did notice: * The unsafety of diff_at_path has been pushed down into known unsafe code. * The unsafe functions from Diff.lhs now document their unsafety in their names. I think this essentially means that we need to be really careful about the code that is in Diff.lhs but we can relax a bit with some of the other code because the GADT types can do their thing to ensure proper sequencing. It certainly seems like you have a good start. Thanks! Jason _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
