[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