[darcs-devel] [patch1913] WIP: get rid of Invert instances for non-prim patch types
Ganesh Sittampalam
bugs at darcs.net
Wed Sep 25 18:27:41 UTC 2019
Ganesh Sittampalam <ganesh at earth.li> added the comment:
> * remove patch type parameter to RebaseName
I remembered one reason why the type parameter was useful:
commuteNamePrim can now be used unsafely to commute a RebaseName with
any patch including a Named.
However this is also a criticism of the existing
commuterIdNamed/commuterNamedId which could be combined to give a wrong
commute for Named/Named. I have patches lying around my WIP to add
comments to each to explain this.
The "right" fix would be to have independent witnesses for names, but
it's unlikely to be worth the disruption to introduce that.
> * RebaseFixup take a prim as type argument, not a RepoPatch
Nice to see we can make this change independently of others.
> * unify RebaseChange and RebaseSelect
This is an excellent simplification.
The comment on getLogInfoCore is now wrong:
> -- This function is split out from getLogInfo/getLogInfoFL
and I guess it could be merged back into getLogInfo. But not too
important to do immediately.
If the introduction of LogInfo had been done in a separate patch, it'd
have been a bit easier to review, and it'd have been clear that the
actual code change to switch RebaseChange is pleasingly simple.
> * get rid of the last Invert constraints for RepoPatch in rebase
Fine
> * remove Invert from RepoPatch constraint synonym
> This also disables tests for inverses of RepoPatches and fixes the
> permutivity test to no longer require it.
I guess we should just delete those test references rather than having
them commented out? (I guess the underlying tests are still there and
used for prim patches)
> * in D.P.Rebase.Viewing replace complicated constraints with RepoPatch
Fine
> * remove Check and Repair instances in D.P.Rebase.Suspended and D.P.Rebase.Item
>
> These were only needed while we mixed the rebase patch with normal patches.
Great.
Not an issue for this bundle as you were only removing dead code, but I
guess we should think about whether the rebase state should be subject
to checking/repairing. We lost that without noticing (or I've forgotten
any discussion) when rebase was moved to the repository level.
> * remove Apply and PrimPatchBase instances in D.P.Rebase.Suspended
Fine
----------
title: get rid of Invert instances for non-prim patch types -> WIP: get rid of Invert instances for non-prim patch types
__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1913>
__________________________________
More information about the darcs-devel
mailing list