On Wed, Sep 24, 2008 at 04:28:50PM +0100, Eric Kow wrote:
> This looks right.  Only silly style comments below.
> 
> > -    where flags2inv | UseFormat2 `elem` fs || UseHashedInventory `elem` fs 
> > = [HashedInventory]
> > -                    | otherwise = [Darcs1_0]
> > +    where flags2inv | UseFormat2 `elem` fs = [HashedInventory]
> > +                    | UseOldFashionedInventory `elem` fs = [Darcs1_0]
> > +                    | otherwise = [HashedInventory]
> 
> It looks like we can simplify this to
> 
>     where flags2inv | UseOldFashionedInventory `elem` fs = [Darcs1_0]
>                     | otherwise = [HashedInventory]

No, this fails if --hashed and --darcs-2 are both provided.  I'd
rather have this be robust, since darcs-2 requires HashedInventory.

> Alternatively, we could make it more explicit/verbose
> 
>     where flags2inv | UseFormat2 `elem` fs = [HashedInventory]
>                     | UseHashedInventory `elem` fs = [HashedInventory]
>                     | UseOldFashionedInventory `elem` fs = [Darcs1_0]
>                     | otherwise = flags2inv [HashedInventory]
>
> Which could simplify switching to some hypothetical inventory format...
> or maybe I'm just in YAGNI-land

This seems like needless overkill.

David

Attachment: signature.asc
Description: Digital signature

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

Reply via email to