[darcs-devel] [patch1911] WIP: use Prim patches in rebase toedit

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

> As the discussion has tended to go off in (interesting!) tangents, I
> want to just summarise what I see as the current state of this bundle.

Let me first remark that I am positively surprised that you got to the
point where the failing-issue2634 no longer fails, in particular it
passes the three-way conflict tests in there, which I must admit I had
doubts a rebase with prim fixups could handle at all. I stand corrected.

> The two biggest things to do are:
> 
>  - Deciding on the backwards compatibility story (being discussed in a
> separate thread on darcs-devel) and then implement it. Once we know the
> strategy, I can update "* WIP: use RebaseChange inside Suspended" to
> include migration. It won't give us the final format, but I think we can
> reasonably regard the interim format it will create as something that we
> won't need to keep supporting.

I haven't yet looked at the individual patches of your bundle, just the
end result. So I cannot usefully comment beyond what I already said
about compatibility.

>  - Finalise fullUnwind so we can decide on RebasePatch. I'm working on
> this at the moment, i.e. working on a clean implementation of the
> squashing of adjacent unwinds, and doing the testing you suggested.

Okay. I wonder... will squashing adjacent unwinds make a lot of
difference to the rebase code?

BTW, your refactors have indeed made that code a lot more modular,
though at the price of making type signatures more complex. The
PushFixupFn abstraction is really quite useful to get a handle on that.

> Apart from any general design discussions, I think we can also usefully
> now think about the type names we want to have. With my refactoring as
> it stands at the moment, we have:
> 
> Suspended - the container patch. Rename to RebaseContainer?
> RebaseChange - an individual suspended patch. Rename to Suspended?
> RebaseItem - backwards compatibility only
> RebasePatch - if we go with it, holds individual suspended prims plus
> their fixups
> RebaseFixup - if we go with RebasePatch, then this is just used in APIs
> and could probably be removed. If we don't go with RebasePatch then it's
> part of RebaseChange.

I think the renames would be a good thing. I like RebasePatch and I
think RebaseFixup is only needed for compatibility (if at all).

Let me finally mention that rebase reify is not yet implemented, it
currently throws an error. Since we found out that reify can be useful
to convert an existing rebase state to normal patches and then back to
another rebase state, I guess it is something we may want to keep after
all, and perhaps turn into an officially supported command.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1911>
__________________________________
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pEpkey.asc
Type: application/pgp-keys
Size: 4211 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-devel/attachments/20191003/ba6774e8/attachment-0001.key>