Excerpts from Jason Dagit's message of Tue Sep 02 22:37:48 +0200 2008:
> On Tue, Sep 2, 2008 at 12:54 PM, <[EMAIL PROTECTED]> wrote:
>
> My attempt at reviewing....
>
> > Tue Sep 2 03:04:05 CEST 2008 [EMAIL PROTECTED]
> > * Change type of subdir parameter in Cache/HashedIO functions from String
> > to HashedDir.
> >
> > This refactor should make calling the Cache and HashedIO functions safer:
> > you
> > should be no longer able to swap hash and subdir accidentally in the call
> > site,
> > or mistype the subdirectory name.
>
> That sounds positive.
>
> But, I wonder about this hunk:
> hunk ./src/Darcs/Repository/HashedIO.lhs 70
> -}
>
> data HashDir r p = HashDir { permissions :: !r, cache :: !Cache,
> - options :: ![DarcsFlag], rootHash :: !String,
> - hashDir :: !String }
> + options :: ![DarcsFlag], rootHash :: !String }
> type HashedIO r p = StateT (HashDir r p) IO
>
> Why did you remove the hashDir from the record? Why did you change
> hashDir :: HashedDir?
>
> > Tue Sep 2 20:57:45 CEST 2008 [EMAIL PROTECTED]
> > * Refactor Cache's handling of hashed paths. No functional change.
> >
> > Factored out the filepath building to a single place. This also led to
> > folding
> > the explicit pattern matches on writability into a predicate, since the
> > other
> > components of a CacheLoc are no longer useful in the function bodies.
>
> This seems fine to me.
>
> >
> > Tue Sep 2 21:35:59 CEST 2008 [EMAIL PROTECTED]
> > * Use qualified imports and shorter names on Cache functions. No
> > functional change.
>
> I like qualified imports just fine. I think Data.ByteString sets a
> good example here. We don't do it much in darcs currently, but I
> think I like this change.Non-ambiguous-non-qualified names are fine too IMHO. They are more traceable, you can jump to the import hyper-quickly and now where it comes. ByteString and Map are a good examples, but I prefer not generalize too much. Just my 2 cents -- Nicolas Pouillard aka Ertai
signature.asc
Description: PGP signature
_______________________________________________ darcs-users mailing list darcs-users@darcs.net http://lists.osuosl.org/mailman/listinfo/darcs-users
