3 patches for repository http://darcs.net/screened:

patch 9b3de1c406a8fd768d34b663f6577fd9b2e613d3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed May 16 09:40:50 CEST 2018
  * remove commuteFLorComplain and replace with commuteFL
  
  The extra information provided by commuteFLorComplain was nowhere used.

patch 20354fcfc3d9695b6a6652e1ae9f810cdb8fb824
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Mar  1 23:56:01 CET 2018
  * move tryShrinkingInverse to D.P.Invert, rename to dropInverses
  
  The function is defined solely in terms of Invert and Eq2.

patch 3da30f7bd97167b3c876255aa01999157e36253c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat May  5 08:58:06 CEST 2018
  * add class PrimSift so we can avoid using PrimConstruct and
PrimClassify in Pending

Attachments

• remove-commuteflorcomplain-and-replace-with-commutefl.dpatch

I talked about this on the mailing list. It is a first step on the way
toward making Darcs work with primitive patch types other than Prim.V1.

I am screening it because my later refactorings and bug fixes regarding
pending depend on this refactor; in fact would not have been possible
without the simplification it makes to D.R.Pending.

>   * remove commuteFLorComplain and replace with commuteFL

Fine

>  * move tryShrinkingInverse to D.P.Invert, rename to dropInverses

Fine. 'dropInitialInverses' might be a better name for this.

>  * add class PrimSift so we can avoid using PrimConstruct and
PrimClassify in Pending

I assume this is just a reorganisation of the code, no functionality 
changes? I haven't checked that line by line.

Why aren't the 'defaultV1' functions just placed directly in the V1 
implementation?

>>  * move tryShrinkingInverse to D.P.Invert, rename to dropInverses
>
> Fine. 'dropInitialInverses' might be a better name for this.

(I was thinking that "drop" implies the "initial" part, by association
with the "drop" function on lists.)

>>  * add class PrimSift so we can avoid using PrimConstruct and
> PrimClassify in Pending
>
> I assume this is just a reorganisation of the code, no functionality 
> changes? I haven't checked that line by line.

Yes, exactly, copy & paste ;-)

> Why aren't the 'defaultV1' functions just placed directly in the V1 
> implementation?

Because they do not depend on Prim.V1, at least not directly. They only
use PrimClassify and PrimCanonize which are nominally not specific to
Prim.V1 (though in practice they are pretty much tied to it). The
functions are used to implement PrimSift for V2.Prim as well as V1.Prim,
both of which are Prim.V1 in disguise, so perhaps it might make sense to
define an instance for Prim.V1 directly and derive the instances for
V1.Prim and V2.Prim. (Originally I wanted these definitions to be
default implementations of the methods but that failed miserably with
numerous typing problems.)

The names of the functions are ugly and I think that's how it should be,
given that this is not an API that was properly designed and there is no
specification for them. In fact, the way isSimple and crudeSift are used
in D.R.Pending is merely for optimization. And this sort of optimization
is really bad because it obscures the code considerably and buys us
almost nothing, since pending is normally rather small. In my later
refactorings of D.R.Pending I have dropped their use, so that only
siftForPending remains. Cleaning up its implementation and perhaps
giving it some sort of spec is left for future work.

Following up on patch review for

patch 3da30f7bd97167b3c876255aa01999157e36253c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat May  5 08:58:06 CEST 2018
  * add class PrimSift so we can avoid using PrimConstruct and
PrimClassify in Pending

4 patches for repository http://darcs.net/screened:

patch aad376b140e5a27ce75498341e5e2083d5852d52
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug 27 11:36:41 CEST 2018
  * move code from D.P.Prim.Sift to D.P.Prim.V1

patch 8d8671ff0ab60d563f26aa4206e565ae6968d023
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug 27 18:50:39 CEST 2018
  * simplify definition of local function sift

patch e03ac4ca9bbf06e01436d285cc1dbee2b3ebda39
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug 27 18:55:28 CEST 2018
  * simplify v1siftForPending
  
  Using the Maybe monad here is completely unnecessary.

patch a233f0f78560692f0d2675ee37d9fe82bccbdbce
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Aug 27 18:55:49 CEST 2018
  * improve docs for crudeSift and v1siftForPending

Attachments

• move-code-from-d_p_prim_sift-to-d_p_prim_v1.dpatch
• patch-preview.txt
• unnamed

I am screening the patches of the bundle I attached in msg20274 in
response to review.

> patch aad376b140e5a27ce75498341e5e2083d5852d52
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Mon Aug 27 10:36:41 GMT Summer Time 2018
>   * move code from D.P.Prim.Sift to D.P.Prim.V1
>
> patch 3da30f7bd97167b3c876255aa01999157e36253c
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat May  5 07:58:06 GMT Summer Time 2018
>   * add class PrimSift so we can avoid using PrimConstruct and 
PrimClassify in Pending

Both fine (it's now easier to review the two in combination)

> patch 8d8671ff0ab60d563f26aa4206e565ae6968d023
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Mon Aug 27 17:50:39 GMT Summer Time 2018
>   * simplify definition of local function sift

Fine

> patch e03ac4ca9bbf06e01436d285cc1dbee2b3ebda39
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Mon Aug 27 17:55:28 GMT Summer Time 2018
>  * simplify v1siftForPending
>
>  Using the Maybe monad here is completely unnecessary.

Fine

> patch a233f0f78560692f0d2675ee37d9fe82bccbdbce
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Mon Aug 27 17:55:49 GMT Summer Time 2018
>   * improve docs for crudeSift and v1siftForPending

Fine