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.

Attachments

• reduce-scope-of-darcs_patch_choices_pull_first_.dpatch
• unnamed

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.

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.

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.

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.

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.

Kamil's comments here: 
http://lists.osuosl.org/pipermail/darcs-users/2009-December/022678.html

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.

I think this is obsoleted by Florent's wider-ranging refactoring 
(patch185).