Hi Kevin (and all),

> Sun Feb  4 11:13:01 MST 2007  Kevin Quick <[EMAIL PROTECTED]>
>   * Added --complement to pull to allow "exclusion" repos
> 
> Tue Feb  6 00:18:32 MST 2007  Kevin Quick <[EMAIL PROTECTED]>
>   * Resolve conflict between complement add and get_recorded_unsorted.

I'm going to accept these since the feature does not seem itself
uncontroversial and any bugs it may have seem to self-contained.

WARNING: I don't fully understand the code in patchset_complement
function in depends, although I have looked at it, and  from the
comment, have a rough idea what it's for.

> Mon Feb  5 23:52:36 MST 2007  Kevin Quick <[EMAIL PROTECTED]>
>   * Add pull_compl test; note interesting duplicate repo elimination in docs.

Definitely appreciate the tests.  Thanks!

Unfortunately, I get this error:
chgrec '3iline2.1\nline2.2' inssub2
sed: 1: "3iline2.1\nline2.2
": command i expects \ followed by text

Could you resubmit?

> Mon Feb  5 11:53:29 MST 2007  Kevin Quick <[EMAIL PROTECTED]>
>   * Add changelog entries (file: quick) for pull --complement changes.

Tommy, is this ok with you? I'll just push it in next week unless you
shout.

========================================================================
Eric's random comments
========================================================================

This is code I'm accepting anyway, but I can rarely resist the urge to
nitpick.

Pull.lhs
--------
>   (_, us', them'') <- return $ get_common_and_uncommon (us, them)
>   (_,   _, compl') <- return $ get_common_and_uncommon (us, compl)
>   them' <- return (patchset_complement them'' compl')

>        return $ if Intersection `elem` opts
>                 then (patchset_intersection rs, [])
>                 else if Complement `elem` opts
>                      then (head rs, patchset_union $ tail rs)
>                      else (patchset_union rs, [])

Perhaps this could be made easier to understand with the case () of _ pattern,
something like this

       return $ case () of _ | Intersection `elem` opts -> 
(patchset_intersection rs, [])
                             | Complement   `elem` opts -> (head rs, 
patchset_union $ tail rs)
                             | otherwise                -> (patchset_union rs, 
[])

It is a bit more indentation-intensive, however.  Ideas?

Also, I tend to dislike uses of head, tail and the like.  I guess
there's little risk of something blowing up, given the read_repos _ []
case, but I wonder if there would be an elegant way to avoid this.

Depends.lhs
-----------
> +patchset_complement x1 xs =

I think you could make the code a bit more readable by making
push_to_end an independent function in Depends.

> +    with_partial_intersection x1 (patchset_union [xs]) $
> +        \_common a b -> let common' = (map fst a) `intersect` (map fst b)

Interestingly, GHC does not complain about the unused _common presumably
because of the initial underscore (for example, it *does* complain about
xxxxxxx).  Bug or feature?  In any case, we should probably get rid of
this.

Also, one thing I did not understand is taking the intersection of
(map fst a) and (map fst b).  Perhaps I am assuming wrong, thinking
that a and b (from with_partial_intersection) are disjoint?  Can you
explain this to me?

> pushing_add = liftM2 (++) pushing $ liftM2 (flip (:)) (return []) ep
> before_add = liftM2 (++) before . return . listof . curry id pinfo . actually

This code has made my head explode.  I feel hoist by my own monad-loving
pointfree-liking petard.

Is there any chance that this could be rewritten in a more obvious
fashion?  Any comments from the list?  I don't want to start any style
debates or anything; maybe this is the ideal form because of its
compactness.  But maybe there are some simplifications lurking in this
code.

(The monad in question is Either MissingPatch, as far as I understand)

> ind_ps  = listof . reverse
> listof  = flip (:) []

Speaking of simplification, it seems like these helpers could be
rewritten as

ind_ps x = [reverse x]
listof x = [x]

best_practices.tex
------------------
> +\section{Personal development}
> +
> +It's easy to have several personal development trees using darcs, even
> +when working on a team or with shared code.

I was a bit doubtful about this update to the best practices being
included in this patch.  I finally negotiated myself into accepting it
by thinking that sophisticated mulit-repo configurations are just the
kind of thing --complement is useful for.

-- 
Eric Kow                     http://www.loria.fr/~kow
PGP Key ID: 08AC04F9         Merci de corriger mon français.

Attachment: pgpsLZ0ZonDjZ.pgp
Description: PGP signature

_______________________________________________
darcs-devel mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-devel

Reply via email to