[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