[darcs-devel] [patch969] break out Darcs.Patch.Rebase.Select module (and 6 more)

[Owen Stephens]

Owen Stephens <darcs at owenstephens.co.uk> added the comment:

I've reviewed this and the original two rebase patches
http://bugs.darcs.net/patch920.

I only seem to have trivial comments, maybe this is a good thing!

- code: commuterRebasing - why is the Suspended :> Suspended case not using
  impossible, if it shouldn't happen?

- trivial: perhaps use Suspended ps/qs rather than Suspended p/q, to give a
  hint that it's a sequnce.

- hmm: The Repair classes are all a bit unexplained and wtf-y

- code: Why are the 2 cases for unsafeCompare commented out? They are
covered
  by the wildcard cases for the Add/Del, no?

- code: Does the ordering in the definition of MyEq for RebaseSelect matter?
  (one checks fixups, edit, the other in the reverse order)

- typo: error case for Rename of mkDummy says it's a delete.

- code: desc in mkReified could just be []?

- hmm: What are the implications of calling revertRepositoryChanges in
  Repository/Rebase?

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch969>
__________________________________