> If this is desired for easier review I can spend some effort to
> split the first patch up into smaller parts.
I've reviewed it as it stands, it was quite easy as it was really
only a few easy-to-follow changes in the type signatures (unless I
missed some significant code changes?).
Perhaps it could have been clearer still with the different changes
broken apart, but certainly not worth the effort now.
Go head and push it (either with or without amending - the patch
description typo is unimportant and the comment typo can be fixed in
a followup if easier).
> Author: Ben Franksen <ben.franksen@online.de>
> Date: Fri Jul 19 13:53:42 CEST 2019
> * clean up the Matchable mess
>
> This is a large refactor. Matchable is no longer a class but a
> constraint
> synonym. This measn we don't have to define instances and can
> therefore
> defined it in Darcs.Patch.Match. Darcs.Patch.Matchable has been
> deleted.
Typo in the patch description ('measn').
> MatchFun, Matcher, and PatchSetMatch no longer get the patch
> type or repo
> type as parameter. Instead, MatchFun is now a data type that
> contains the
> polymorphic match function (constrained to work only on
> Matchable patches).
This seems like a nice change as it gets rid of annoying type
parameters, and the existence of Dummy to instantiate it was an
indication that something wasn't quite right.
On the other hand it will stop us writing matchers specific to a
single patch type in future, but that's very hypothetical.
> Matchable p now also requires Ident p and PatchId p ~ PatchInfo,
> so we have
> access to the PatchInfo (using ident). The goal here was to
> generalize
> matching functions from PatchInfoAnd to any Matchable patch
> type. Since some
> of the matching functions work on PatchSets, which have
> PatchInfoAnd
> hard-coded, we add a new constraint synonym for RepoPatches p
> such that
> PatchInfoAnd rt p becomes Matchable.
>
> Darcs.UI.SelectPatches no longer has PatchInfoAnd in any of its
> type
> signatures.
> hunk ./src/Darcs/Patch/Match.hs 137
[..]> -- | Constraint for a patch type @p@ that ensures
@'PatchInoAnd' rt p@
Typo in the doc comment ('PatchInoAnd')
> hunk ./src/Darcs/UI/SelectChanges.hs 1008
> -askAboutDepends :: (IsRepoType rt, RepoPatch p, ApplyState p ~
Tree)
> +askAboutDepends :: ( IsRepoType rt, RepoPatch p, ApplyState p ~
Tree )
Not a big deal, but this seems like a gratutious and slightly
negative whitespace change. We don't normally put spaces around one-
line constraints here, do we?
> Author: Ben Franksen <ben.franksen@online.de>
> Date: Mon Jul 29 18:04:27 CEST 2019
> * eliminate Darcs.Patch.Dummy
Nice.
|
