I am not screening this patch immediately as it will most certainly conflict
with your work in progress. However, I would really very much like this to
go into screened ASAP.

1 patch for repository http://darcs.net/screened:

patch 04aa87d4f01782ebb0b03318c21d31ab69b49aa6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Feb 16 09:53:32 CET 2020
  * massive boilerplate reduction in test harness
  
  Repeating instance definitions ad nauseam is not good for our health. This
  patch makes a number of invasive refactors in the test harness that
  dramatically reduce that boilerplate. Here are the main ideas:
  
  Having Arbitrary instances for both Sealed and Sealed2 is stupid. For
  testing we only need Sealed2. So the generators and infrastructure have been
  refactored to always take and generate Sealed2 patches. This has the
  beneficial side-effect of cleaning up a lot of the types in the testing
  infrastructure.
  
  For most of the remaining Arbitrary instances we can provide a single
  generic instance. To make this possible we need to use the generic model
  generator (aSmallRepo from the RepoModel class), which we always do except
  for the RepoPatchV1 tests. My solution for this was to move all the
  RepoPatchV1 tests into a separate module and throw out the tests that are
  disabled for RepoPatchV1 anyway. Even with this out of the way, I needed to
  refactor WithState and the class ArbitraryState to no longer take the
  model/state as parameter, but rather use the type function ModelOf.
  
  Additional minor cleanups:
  - The TestGenerator/TestCondition/TestCheck machinery now lives in the
    D.T.Patch.Utils module.
  - Class ArbitraryStateIn was removed and its single method inlined at its
    single call site.
  - Generalize qc_prim32/3 to qc_named_prim.
  - Removed some redundant constraints.
  - Renamed Darcs.Test.Util.TestResult.fromMaybe to avoid collision with the
    well known Data.Maybe.fromMaybe

Attachments

• massive-boilerplate-reduction-in-test-harness.dpatch
• patch-preview.txt
• unnamed

PS sorry for mixing all this in a single patch. If this hurts a lot I am
willing to spend some effort to split it up.

Attachments

• pEpkey.asc

I think the only substantial thing I have pending for the tests is 
patch1986, which I would like to get in first if I could. I think it
is ready to screen, maybe you can take a quick look and shout if you
see any major problems.

Other than that this would be a lot easier to digest if it was in
smaller logical pieces, but I don't feel too strongly.
I've been thinking about similar simplifications as well, though as
I've mentioned I'm nervous about accidentally messing up coverage
in the process.

I'm not sure about the removal of ArbitraryStateIn; even if it's only
used once, I think it's logically the right abstraction alongside
ArbitraryState for things that only have one witness. That said,
if we're going to replace Tree with MergeableSequence anyway then
maybe it doesn't matter.

> I think the only substantial thing I have pending for the tests is 
> patch1986, which I would like to get in first if I could. I think it
> is ready to screen, maybe you can take a quick look and shout if you
> see any major problems.

Looks good to me. I have already rebased my large refactor on top of
patch1986, no problem.

> Other than that this would be a lot easier to digest if it was in
> smaller logical pieces, but I don't feel too strongly.

I have been doing this as a sort of experiment at first, then as I got
the feeling that this could actually be made to work I couldn't stop
myself...

> I've been thinking about similar simplifications as well,

...and indeed your patches already remove a bit of the boilerplate
instances for the various prim patch types (notably ArbitraryState).

> I'm not sure about the removal of ArbitraryStateIn; even if it's only
> used once, I think it's logically the right abstraction alongside
> ArbitraryState for things that only have one witness.

I'll take another look at it. I never quite got the idea of this class,
but perhaps I just have to try harder.

Anyway, I will screen your patch1986 unless you beat me to it and will
send a rebased version of mine; and screen that, perhaps with a few
minor modifications.

Attachments

• pEpkey.asc

An updated patch, rebased, with improved description and w/o the removal of
class ArbitraryStateIn.

1 patch for repository http://darcs.net/screened:

patch 26ffb61437f21157b4fac3e55c841cf152559f40
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Feb 16 09:53:32 CET 2020
  * massive boilerplate reduction in test harness
  
  This patch makes a number of invasive refactors in the test harness that
  dramatically reduce the boilerplate of repeated instance Arbitrary
  definitions. Here are the main ideas:
  
  First, remove all instances for Sealed patches and keep only the ones for
  Sealed2 patches. The generators and infrastructure have been refactored to
  always take and generate Sealed2 patches. This has the beneficial
  side-effect of cleaning up a lot of the types in the testing infrastructure.
  
  For most of the remaining Arbitrary instances we can provide a single
  generic instance. To make this possible we need to use the generic model
  generator (aSmallRepo from the RepoModel class), which we always do except
  for the RepoPatchV1 tests. My solution for this was to move all the
  RepoPatchV1 tests into a separate module and throw out the tests that are
  disabled for RepoPatchV1 anyway. Even with this out of the way, I needed to
  refactor WithState and the class ArbitraryState to no longer take the
  model/state as parameter, but rather use the type function ModelOf. This,
  too, make the types simpler and signatures less verbose.
  
  Additional minor cleanups:
  - The TestGenerator/TestCondition/TestCheck machinery now lives in the
    D.T.Patch.Utils module.
  - Generalize qc_prim32/3 to qc_named_prim.
  - Removed some redundant constraints.
  - Renamed Darcs.Test.Util.TestResult.fromMaybe to avoid collision with the
    well known Data.Maybe.fromMaybe
  - Allow all darcs-lib extensions for darcs-test, too; cleanup module-local
    extension pragmas.

Attachments

• massive-boilerplate-reduction-in-test-harness.dpatch
• patch-preview.txt
• unnamed

Screening the amended version now.

They aren't needed in GHC 8.2, I didn't check 8.4
1 patch for repository darcs-unstable@darcs.net:screened:

patch b25ff3a8e8d286dedc2a227b11967fdc6e97fd9f
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Wed Feb 19 22:36:03 GMT 2020
  * tests: GHC 8.6 requires a couple of UndecidableInstances

Attachments

• patch-preview.txt
• tests_-ghc-8_6-requires-a-couple-of-undecidableinstances.dpatch
• unnamed

> They aren't needed in GHC 8.2, I didn't check 8.4

Could you add a short comment to each so we (or rather I) don't
accidentally throw them out again?

Attachments

• pEpkey.asc

That's a good point, I would probably also have removed them on sight if 
it built for me locally. Would just identifying the GHC version to check 
with be enough?

> That's a good point, I would probably also have removed them on sight if
> it built for me locally. Would just identifying the GHC version to check
> with be enough?

I think so.

________________________________

Helmholtz-Zentrum Berlin für Materialien und Energie GmbH

Mitglied der Hermann von Helmholtz-Gemeinschaft Deutscher Forschungszentren e.V.

Aufsichtsrat: Vorsitzender Dr. Volkmar Dietz, stv. Vorsitzende Dr. Jutta Koch-Unterseher
Geschäftsführung: Prof. Dr. Bernd Rech (Sprecher), Prof. Dr. Jan Lüning, Thomas Frederking

Sitz Berlin, AG Charlottenburg, 89 HRB 5583

Postadresse:
Hahn-Meitner-Platz 1
D-14109 Berlin

1 patch for repository darcs-unstable@darcs.net:screened:

patch 53431782f9901daf6fd88a667cc2818f466f8eed
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Thu Feb 20 17:34:09 GMT 2020
  * add comments about why UndecidableInstances is needed

Attachments

• add-comments-about-why-undecidableinstances-is-needed.dpatch
• patch-preview.txt
• unnamed

My review of your patches is now deadlocked because they depend on 
this one.

You already commented and even sent your own fixes, so I'll self-
accept.