[darcs-devel] [patch2002] introduce unwind

[Ganesh Sittampalam]

Ganesh Sittampalam <ganesh at earth.li> added the comment:

[apologies, another patch with a bad initial subject, which I've fixed]

>>   * tests: generalise hasPrimConstruct, add usesV1Model
> 
> You may have expected that I won't like this one overmuch ;-)
> 
> I can appreciate the need to move from the Bool to a Dict here and I am
> not objecting to that part. However, having both hasPrimConstruct and
> hasV1Model smells. Logically these are the same statement i.e. we
> require patches based on Prim.V1. Perhaps this is a good time to do
> acknowledge that PrimConstruct really cannot be defined usefully for
> other prim types... anyway, I don't think this should hold up screening
> these patches, just add a TODO marker here so we remember we should
> clean this up.

I've added the TODO. I'm not sure we should just explicitly look for
Prim.V1 though. For example there are the newtype wrappers for
(Repo)V1/V2. I also have some old experiments with making Prim.V1 more
general/modular, that I'd expect to be able to implement PrimConstruct
or something like it.

Still, I fully agree that the PrimConstruct class is very specific. I
introduced that and PrimClassify mainly to make it possible to overload
on prim types at all, not with the feeling that they were the one true
design.

>>   * tests: add withAllSequenceItems
> 
> (1) If you recognise that combineTestResults is just mconcat with a
> suitable SemiGroup instance I would expect you to replace this function
> with mconcat everywhere.

Yes, true, sent.

Though I now have slight second thoughts: are we happy that the instance
is canonical enough> I think it probably is but there are probably other
possible instances, such as rejecting if anything is rejected rather
than just if everything is rejected.

> (2) I get a warning about missing implementation of mappend (ghc-8.2). I
> think you should define mappend instead of (<>).

Sorry, fixed. I think we need mappend in Monoid and <> in Semigroup for now.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch2002>
__________________________________