Patch 35 Reduce scope of Darcs.Patch.Choices.pull... (and 1 more)

Title Reduce scope of Darcs.Patch.Choices.pull... (and 1 more)
Superseder Nosy List Kamil, ganesh, kowey, mornfall
Related Issues darcs changes -s should show both adds andremoves
View: 185
Status obsoleted Assigned To ganesh

Created on 2009-10-31.21:05:49 by kowey, last changed 2010-04-03.12:58:59 by ganesh.

File name Status Uploaded Type Edit Remove
reduce-scope-of-darcs_patch_choices_pull_first_.dpatch kowey, 2009-10-31.21:05:48 text/x-darcs-patch
unnamed kowey, 2009-10-31.21:05:48 text/plain
See mailing list archives for discussion on individual patches.
msg9148 (view) Author: kowey Date: 2009-10-31.21:05:48
Sun Oct 11 17:07:44 BST 2009  Eric Kow <kowey@darcs.net>
  * Reduce scope of Darcs.Patch.Choices.pull_first.
  Pull_firsts makes use of it and seems to discard some of the intermediary
  results.  I want to ensure that changing these intermediary results does
  not have unintended consequences.

Sun Oct 11 19:09:47 BST 2009  Eric Kow <kowey@darcs.net>
  * Remove completely superfluous negTag mechanism.
  This change is really due to Kamil Dworakowski, who observed that pull_firsts
  returns all the InFirst patches along with the InMiddle patches they depend on
  without any tags.
msg9173 (view) Author: kowey Date: 2009-11-01.23:49:16
Hi Petr, could you have a quick look at this one please?  Might be a good chance
to see the patch selection code, which I don't remember if you've already played
with much.
msg9620 (view) Author: ganesh Date: 2009-12-14.12:11:19
Grabbing this as it's been sitting around for a while.

The scope reduction looks fine.

I'm less convinced about the removal of negTag. As far as I can see, the negated
tags do end up being returned from pull_firsts, because the next call to
pull_first will have the patch with the negated tag at the head. So we'd need to
be sure that it didn't actually matter that we were no longer negating the tags.

Another reason to be concerned is that I suspect the test coverage of
interactive selection is minimal.
msg9624 (view) Author: ganesh Date: 2009-12-14.12:19:20
Actually, the scope reduction patch generates "shadowing" warnings.

I'm never particularly convinced that these warnings are that worthwhile, so one
option would be to turn them off.
msg9666 (view) Author: kowey Date: 2009-12-23.16:27:08
If it helps, here is the mail which (after some thinking) convinced me that this
was safe:
- http://lists.osuosl.org/pipermail/darcs-users/2009-September/021554.html

Although now having not though about it recently, I'm nervous again.
msg9695 (view) Author: kowey Date: 2009-12-28.15:07:48
Hi Kamil,

Do you think you could comment about this?  It's about that negTag in
Darcs.Patch.Choices.  It's nothing urgent (we're focused on getting Darcs 2.4
out the door anyway), but if I may propose this is a todo list candidate, that'd
be fine.

If you don't think you'll have time for this, that's not a big deal.  We could
also just reject this patch and move on to other things, maybe a better use of time.
msg9762 (view) Author: kowey Date: 2010-01-09.18:28:22
Kamil's comments here: 

I'd like to see negTag go if we can be fairly certain it's just cruft, but maybe
we should just set ourselves some sort of deadline, for example to reject this
patch by next month.
msg10651 (view) Author: ganesh Date: 2010-04-03.12:58:59
I think this is obsoleted by Florent's wider-ranging refactoring 
Date User Action Args
2009-10-31 21:05:49koweycreate
2009-11-01 23:49:16koweysetassignedto: mornfall
messages: + msg9173
nosy: + mornfall
2009-11-02 17:21:02koweysetstatus: needs-review -> review-in-progress
2009-12-14 11:37:42ganeshsetassignedto: mornfall -> ganesh
nosy: + ganesh
2009-12-14 12:11:22ganeshsetmessages: + msg9620
2009-12-14 12:19:20ganeshsetmessages: + msg9624
2009-12-23 16:27:08koweysetmessages: + msg9666
2009-12-28 15:07:49koweysetnosy: + Kamil
messages: + msg9695
2010-01-09 18:28:23koweysetmessages: + msg9762
2010-04-03 12:58:59ganeshsetstatus: review-in-progress -> obsoleted
messages: + msg10651
issues: + darcs changes -s should show both adds andremoves