[darcs-users] darcs patch: Change type of subdir parameter in Cache... (and 2 more)

[Nicolas Pouillard]

Excerpts from Jason Dagit's message of Tue Sep 02 22:37:48 +0200 2008:
> On Tue, Sep 2, 2008 at 12:54 PM,  <me at mornfall.net> wrote:
> 
> My attempt at reviewing....
> 
> > Tue Sep  2 03:04:05 CEST 2008  me at mornfall.net
> >  * 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  me at mornfall.net
> >  * 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  me at mornfall.net
> >  * 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20080902/08d1a55c/attachment-0001.pgp