darcs

Patch 149 Give Darcs.SelectChanges.text_view a san... (and 5 more)

Title Give Darcs.SelectChanges.text_view a san... (and 5 more)
Superseder Rename filterFL to filterFLFL, replace i... (and 5 more)
View: 155
Nosy List darcs-users, galbolle, ganesh
Related Issues
Status obsoleted Assigned To ganesh
Milestone

Created on 2010-02-01.19:21:33 by galbolle, last changed 2011-05-10.22:06:18 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
give-darcs_selectchanges_text_view-a-sane-type.dpatch galbolle, 2010-02-01.19:21:32 text/x-darcs-patch
unnamed galbolle, 2010-02-01.19:21:32 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg9914 (view) Author: galbolle Date: 2010-02-01.19:21:32
These patches are for post-2.4, they add witnesses to Darcs.Commands.Changes.
The main change is that PatchSet now has two type witnesses: we allow
"partial PatchSets", which don't start at the initial (empty) state of the
repo. A PatchSet start x goes from 'start' to 'x', and any tag appearing
beyond the first RL in it is clean. The type 'Origin' is used as a witness
to specify which functions need a PatchSet to be full, or yield a full
PatchSet ( () used to be used for this, but I think it's not as clear).

Hopefully, this means a fully witnessed darcs is coming…

On a quite not unrelated note, do we have a roadmap for darcs 2.5?

Florent

6 patches for repository http://darcs.net:


Fri Jan 29 10:58:22 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Give Darcs.SelectChanges.text_view a sane type

Fri Jan 29 17:20:00 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * add seals in Changes to prepare for witnesses

Sun Jan 31 17:25:39 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Remove useless case in Patch.Bundle

Sun Jan 31 22:55:19 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Allow PatchSets not to start at the origin of the repo

Wed Jan 27 14:19:24 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Rename filterFL to filterFLFL, replace it with a version using Sealed2

Mon Feb  1 01:15:47 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
  * Add witnesses to Darcs.Commands.Changes
Attachments
msg9917 (view) Author: ganesh Date: 2010-02-01.22:52:16
On Mon, 1 Feb 2010, Florent Becker wrote:

> These patches are for post-2.4, they add witnesses to 
> Darcs.Commands.Changes. The main change is that PatchSet now has two 
> type witnesses: we allow "partial PatchSets", which don't start at the 
> initial (empty) state of the repo. A PatchSet start x goes from 'start' 
> to 'x', and any tag appearing beyond the first RL in it is clean. The 
> type 'Origin' is used as a witness to specify which functions need a 
> PatchSet to be full, or yield a full PatchSet ( () used to be used for 
> this, but I think it's not as clear).

Even if someone else reviews this too, I'd like to take a look before it 
goes in, if that's ok. I might not be able to look before the weekend, but 
will definitely do so then.

One immediate question: what does it mean for a tag to be "clean" in a 
PatchSet that doesn't start from Origin? I can roughly guess what it means 
operationally (the PatchSet was dissected from a PatchSet where the tag 
was clean), but I'm a bit concerned that it won't have any direct meaning 
for the new PatchSet. It leaves me wondering whether it does make sense to 
reuse the PatchSet concept in this situation.

Cheers,

Ganesh
msg9960 (view) Author: ganesh Date: 2010-02-06.22:07:13
> Fri Jan 29 10:58:22 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>  * Give Darcs.SelectChanges.text_view a sane type

Definitely looks good

> Fri Jan 29 17:20:00 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>   * add seals in Changes to prepare for witnesses

This one looks good too. The basic point is that we are now giving up on 
witnesses for viewing changes, which makes reasonable sense given that 
(a) it's a low-risk activity and (b) filtering changes makes witnesses a 
nightmare.

Along the way, the patch gets rid of the use of TaggedPatch in text_view 
in Darcs.SelectChanges, which seems appropriate as the tags don't seem 
to have been used anywhere.

> Sun Jan 31 17:25:39 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>   * Remove useless case in Patch.Bundle

Looks ok, do we know how the code got into this state?

> Sun Jan 31 22:55:19 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>  * Allow PatchSets not to start at the origin of the repo

As per my previous email, I'm still not convinced by this. It changes a 
lot of code for one particular use case (which I guess is in the patch 
adding witnesses to Changes, which I haven't read carefully yet). Is it 
really worth it?

> Wed Jan 27 14:19:24 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>  * Rename filterFL to filterFLFL, replace it with a version using 
Sealed2

This cleans up a confusion introduced by the addition of filterRL 
earlier: the type/usage of the newly added filterRL wasn't at all 
consistent with the existing filterFL.

> Mon Feb  1 01:15:47 CET 2010  Florent Becker <florent.becker@ens-
lyon.org>
>  * Add witnesses to Darcs.Commands.Changes

I haven't read this carefully yet, but I didn't spot any obvious 
problems on a quick glance.
msg9963 (view) Author: ganesh Date: 2010-02-07.22:22:47
> Mon Feb  1 01:15:47 CET 2010  Florent Becker <florent.becker@ens-lyon.org>
>  * Add witnesses to Darcs.Commands.Changes

This one looks fine too. One minor comment is that you've left in my
comment in witnesses.hs which is intended to explain why modules aren't
yet witnessed and is therefore no longer relevant.

Also, your patch series seems to have conflicts. If it's difficult for
you to amend, it's not a big deal, but otherwise it might be nice to do so.

So I think the only significant issue is that I'd like to discuss the
extra type argument to PatchSet further before accepting these patches.
msg9990 (view) Author: ganesh Date: 2010-02-09.20:42:32
I talked to Florent on IRC and he convinced me about the extra parameter
to PatchSet: http://irclog.perlgeek.de/darcs/2010-02-08#i_1967852
History
Date User Action Args
2010-02-01 19:21:33galbollecreate
2010-02-01 19:24:07darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-eb13e829a64020f1901d30fd8a2927afb69a3b3a
2010-02-01 22:52:17ganeshsetnosy: + ganesh
messages: + msg9917
2010-02-06 19:16:25ganeshsetstatus: needs-review -> review-in-progress
assignedto: ganesh
2010-02-06 22:07:14ganeshsetmessages: + msg9960
2010-02-07 22:22:48ganeshsetmessages: + msg9963
2010-02-09 20:41:09ganeshsetstatus: review-in-progress -> obsoleted
superseder: + Rename filterFL to filterFLFL, replace i... (and 5 more)
2010-02-09 20:42:32ganeshsetmessages: + msg9990
2011-05-10 22:06:18darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-eb13e829a64020f1901d30fd8a2927afb69a3b3a -> http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-eb13e829a64020f1901d30fd8a2927afb69a3b3a