On Wed, Jan 07, 2009 at 20:57:56 +0100, Petr Rockai wrote:
> Ok, looks fine to me. Please consider the inlined comments below. Comments or
> not, please hurry to apply this to mainline (I mean, http://darcs.net) and 
> I'll
> pull it into release branch. A second review would be good, too, if there are
> takers.

Hurriedly applied!  Thanks

> exist1 and exist2 may not be the best variable names ever... :) I agree with
> the minimal change for this, but it might be worth recording a patch on top
> that factors out the doesFileExist || doesDirectoryExist idiom.

Yeah.  There might be some kind of monadic mumbo jumbo somewhere to make
this nice and natural (liftM2 (||))?

> This change is a little confusing to me. The original meaning is (due to tight
> binding of not) "the file is not recorded or it is pending" whereas the new is
> "the file is not recorded nor pending". Indeed, the file is likely in a
> repository if it's either recorded or pending.

Christian seems to agree

> Again, further refactoring might make sense -- is this the only place
> where we check whether a file is in repository? What about Changes.lhs
> (I haven't checked... now I see that darcs changes foobarh won't issue
> a warning... maybe we should add one...)?

Worth looking into for after the release.

Thanks!

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Attachment: signature.asc
Description: Digital signature

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

Reply via email to