darcs

Patch 1775 Assorted collection of minor code cleanups (18 patches)

Title Assorted collection of minor code cleanups (18 patches)
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2018-12-04.17:24:51 by bfrk, last changed 2019-06-14.12:09:36 by ganesh.

Files
File name Status Uploaded Type Edit Remove
darcs_util_tree_hashed_-add-debug-messages-and-factor-out-showhash.dpatch bfrk, 2018-12-04.17:24:50 application/x-darcs-patch
patch-preview.txt bfrk, 2018-12-04.17:24:50 text/x-darcs-patch
unnamed bfrk, 2018-12-04.17:24:50 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg20573 (view) Author: bfrk Date: 2018-12-04.17:24:50
Assorted collection of minor code cleanups accumulated over the past few
weeks. None of them make any substantial change to the code base; for some
of the layout changes you'll have to trust me on that. The rest is mostly
removing unreferenced code, some of which was in the way of more substantial
changes I'll send separately.

18 patches for repository http://darcs.net/screened:

patch cc5961887b87f80a9518ccfd2014a52f46753224
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Dec  4 14:14:37 CET 2018
  * Darcs.Util.Tree.Hashed: add debug messages and factor out showHash

patch 8e33672608c1ea62423dbe26c8ee394f42661369
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Oct 18 10:54:02 CEST 2018
  * remove unused method effectRL from class Effect

patch 135f1149ed48f28f967a77b8b26aa421f65e7663
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Oct 18 12:26:51 CEST 2018
  * removed unneeded pseudo-instances for class Effect

patch 13009bb43b0324b3bfee61f57dd8d981550db7c5
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Nov 12 10:07:05 CET 2018
  * remove unused option optimizePatchIndex
  
  This option became a subcommand some time ago.

patch 5249e5fb34ee4a8330ce68034d09f172208246d5
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Nov 12 10:12:18 CET 2018
  * group --reorder under the merge options and improve its help text

patch 5aae2881494d7fd808998095eece7457ae745644
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Nov 13 11:19:57 CET 2018
  * move removeFromTentativeInventory closer to its single call site

patch d60c4da4309311bc5e96e2fcc7fcd2d7e0b114b7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Nov 13 17:54:14 CET 2018
  * Darcs.Repository.Clone: turn two comments into debug messages

patch 8a8a98865c1935723a2d6a207b0f3334c8d94560
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Nov 13 17:54:20 CET 2018
  * Darcs.Repository.Clone: fix a comment

patch b80d42182484ff4e5da87f9b5d4d807f80587a66
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Nov 14 22:05:02 CET 2018
  * D.R.Merge: use displayPath for path display

patch a0b70ded00a2b18d23caf4b8340a0697e17f1ad6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jul 27 10:35:06 CEST 2018
  * remove outdated haddocks for revertRepositoryChanges

patch ad6e238ea20eafdf0efde0edd38403b2de024d34
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 11 20:47:30 CET 2018
  * remove unused functions choosePreTouching and selectTouching

patch 15a1c521c7c350520b236cbab000bf88e72bd88d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 15 18:08:21 CET 2018
  * Darcs.Repository.Clone: clean up imports from D.R.Hashed

patch a25111c7450d2ae353d522ea043e9e513ea5b797
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 15 20:57:35 CET 2018
  * revert command: break overlong lines (layout only)

patch 94cd11f184fde69473a7190277ffd103775333dc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 15 22:18:47 CET 2018
  * remove duplicate definition of catchall in Darcs.UI.External
  
  Instead use the one from Darcs.Util.Exception.

patch 8195885a2f47c7bb10a1c67bb4ea972ef0d0cf3b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Nov 16 09:42:17 CET 2018
  * layout in D.UI.C.Amend.addChangesToPatch

patch 576dd8b689d308c1761e5c9eda03d04fa6345fc9
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Nov 16 10:01:56 CET 2018
  * slightly simplify withManualRebaseUpdate

patch e587d3a8837626433b65209472f8140f90a79a42
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Dec  3 16:04:53 CET 2018
  * remove unused/obsolete src/win32/sys/mman.h

patch 3c4fbcecce288a97c866d55955913f24d743d776
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 25 14:33:50 CET 2018
  * layout changes to avoid overlong lines in Darcs.Util.Tree.Monad
Attachments
msg20671 (view) Author: ganesh Date: 2019-06-02.18:28:59
>  * Darcs.Util.Tree.Hashed: add debug messages and factor out 
showHash

OK

>  * remove unused method effectRL from class Effect

OK

>  * removed unneeded pseudo-instances for class Effect

OK

>   * remove unused option optimizePatchIndex

OK
msg20672 (view) Author: ganesh Date: 2019-06-02.18:37:10
>  * group --reorder under the merge options and improve its help 
text

> +  [ RawNoArg [] ["reorder-patches"] F.Reorder Reorder
> +    "put local-only patches on top of remote ones"
> +  , RawNoArg [] ["no-reorder-patches"] F.NoReorder NoReorder
> +    "put remote-only patches on top of local ones" ]

I think the comment about NoReorder is a bit misleading. Reorder
explicitly moves all local-only patches to the top of the
repository. But NoReorder will just put the *new* remote patches
at the top of the repository, leaving everything else in the 
original order (which could have been anything).

Regardless, the new comments are much clearer than the old ones.
msg20673 (view) Author: ganesh Date: 2019-06-02.18:39:30
>  * move removeFromTentativeInventory closer to its single call 
site

OK

>   * Darcs.Repository.Clone: turn two comments into debug messages

OK

>   * Darcs.Repository.Clone: fix a comment

OK

>   * D.R.Merge: use displayPath for path display

OK
msg20674 (view) Author: ganesh Date: 2019-06-02.19:03:43
>  * remove outdated haddocks for revertRepositoryChanges
OK

>  * remove unused functions choosePreTouching and selectTouching
OK

>  * Darcs.Repository.Clone: clean up imports from D.R.Hashed

OK

>  * revert command: break overlong lines (layout only)

OK

>  * remove duplicate definition of catchall in Darcs.UI.External

OK


>  * layout in D.UI.C.Amend.addChangesToPatch

OK

>  * slightly simplify withManualRebaseUpdate

OK

>  * remove unused/obsolete src/win32/sys/mman.h

OK (assuming it builds on Windows :-)

>  * layout changes to avoid overlong lines in Darcs.Util.Tree.Monad

OK
msg20712 (view) Author: bfrk Date: 2019-06-11.14:43:57
Am 02.06.19 um 20:37 schrieb Ganesh Sittampalam:
> 
> Ganesh Sittampalam <ganesh@earth.li> added the comment:
> 
>>  * group --reorder under the merge options and improve its help 
> text
> 
>> +  [ RawNoArg [] ["reorder-patches"] F.Reorder Reorder
>> +    "put local-only patches on top of remote ones"
>> +  , RawNoArg [] ["no-reorder-patches"] F.NoReorder NoReorder
>> +    "put remote-only patches on top of local ones" ]
> 
> I think the comment about NoReorder is a bit misleading. Reorder
> explicitly moves all local-only patches to the top of the
> repository. But NoReorder will just put the *new* remote patches
> at the top of the repository, leaving everything else in the 
> original order (which could have been anything).

Good catch. Perhaps this is better:

"place new patches at the end of your repository"

?
msg20740 (view) Author: ganesh Date: 2019-06-12.06:07:30
> Good catch. Perhaps this is better:

> "place new patches at the end of your repository"

Yes, or maybe "put newly fetched remote-only patches at the
top of your repository" to keep the language consistent with the
other case. It's kind of obvious that newly fetched patches
are remote-only, but might not be so intuitive to all users.
msg20747 (view) Author: bfrk Date: 2019-06-12.11:16:56
Re help text for --reorder-patches. I think now that my original 
help texts are correct and not misleading:

reorder: "put local-only patches on top of remote ones"
no-reorder: "put remote-only patches on top of local ones"

Formulating it in this way makes it clear that the two options 
really are symmetric: the uncommon parts of both repos (local-only 
and remote-only, where remote-only includes only the selected 
patches) are either merged "first local then remote", or "first 
remote then local". This is exactly what the implementation does 
(see D.R.Merge).
msg20785 (view) Author: ganesh Date: 2019-06-14.11:34:58
Of course, remote-only patches by definition are new. Sorry for my
muddled thinking.
History
Date User Action Args
2018-12-04 17:24:51bfrkcreate
2018-12-04 17:28:36bfrksettitle: Darcs.Util.Tree.Hashed: add debug messag... (and 17 more) -> Assorted collection of minor code cleanups (18 patches)
2018-12-04 17:42:39bfrksetstatus: needs-screening -> needs-review
2019-06-02 18:29:00ganeshsetmessages: + msg20671
2019-06-02 18:37:10ganeshsetstatus: needs-review -> review-in-progress
messages: + msg20672
2019-06-02 18:39:30ganeshsetmessages: + msg20673
2019-06-02 19:03:44ganeshsetmessages: + msg20674
2019-06-11 14:43:57bfrksetmessages: + msg20712
2019-06-12 06:07:30ganeshsetmessages: + msg20740
2019-06-12 11:16:56bfrksetmessages: + msg20747
2019-06-14 11:34:58ganeshsetstatus: review-in-progress -> accepted-pending-tests
messages: + msg20785
2019-06-14 12:09:36ganeshsetstatus: accepted-pending-tests -> accepted