[darcs-devel] [patch1947] refactor simplifyPush (and 9 more)
Ben Franksen
bugs at darcs.net
Sat Feb 22 07:58:34 UTC 2020
Ben Franksen <ben.franksen at online.de> added the comment:
Sorry for letting this lie around for so long.
> patch 393574cfc5cbccd959c0f9a14c2e9d9eed4f0ab9
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:15:09 BST 2019
> * refactor simplifyPush
>
> This patch introduces a general concept of pushing 'fixups'
> past 'items', and uses it to implement simplifyPush.
>
> This provides a framework for modularizing the logic in
> subsequent patches.
Okay. It took me a bit to realize that the changes to
Darcs.Patch.Rebase.Item are actually quite trivial. Basically, we move
from returning Sealed to returning a pair i.e. we don't immediately
throw away the fixups that come out at the end.
> patch fcc84f7dfb92f769a649bfbdd5ba6aba639f4280
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:19:06 BST 2019
> * drop redundant case in pushFixupItem
I have stared at this for a while and don't get why this case is
redundant. Could you explain?
> patch 17f854155bbacb52bf2ab974e04fcb82a2e56d00
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:19:28 BST 2019
> * drop error in commuteNameNamed if a deleted name is readded
>
> As in D.P.R.Item.pushFixupItem, this scenario isn't impossible
> so should just be a failed commute rather than an error.
Okay, agreed.
> patch 287c8c901e9ba38e018dfeef7c79cf2473edd563
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:21:22 BST 2019
> * reuse commuteNameNamed to push RebaseName through ToEdit
>
> A commute is the natural way of implementing this operation
> and following the previous refactors the logic was identical
Okay.
> patch f7235ff1566651fe2f04dac2a71d103b3edd9435
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:32:01 BST 2019
> * add Maybe2 type
Fine.
> patch 2283cc4f2f6a131728fbcd0a622a9bc49f7458cd
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:32:08 BST 2019
> * move logic for pushing name fixups to D.P.R.Name
Yup that makes sense.
> patch 08e571529264913630083abe368ef355a5d4195b
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:40:03 BST 2019
> * move logic for pushing prim fixups to D.P.R.Fixup
Same here.
> patch 9f3996fbc438da7df101b85535aa760077fbcab3
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:41:40 BST 2019
> * reorder cases in pushFixupItem
>
> Just to make it easier to see how connected logic can be
> extracted to a separate function.
Agreed...
> patch 3e3ebc087fb95171de2fc1ed7befc0b111135289
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:50:20 BST 2019
> * move logic for pushing past fixups into D.P.R.Fixup
...because again this looks good and it is now indeed easier to see that
it is correct.
> patch 7fa75673d2f15d74320b510f557b6d19fdbc004e
> Author: Ganesh Sittampalam <ganesh at earth.li>
> Date: Fri Sep 27 20:53:55 BST 2019
> * use commuteFixupNamed in pushFixupItem
Fine.
If you could respond to the one question above about patch
fcc84f7dfb92f769a649bfbdd5ba6aba639f4280 I'll accept this bundle.
__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1947>
__________________________________
-------------- 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/20200222/79b1adf8/attachment.key>
More information about the darcs-devel
mailing list