Re: multiple merge notifications

On Thu, 10 May 2007, Eric Gillespie wrote:

> "Ben Collins-Sussman" <sussman@red-bean.com> writes:
>
> > Here's a patch in progress, not yet committed, which displays multiple
> > merge-range notifications during a merge.
>
> Looks good!

I immeidately mentioned this on IRC, but for posterity, I also think
that the patch looks good.

> > So, as a stopgap, I've hacked a function in svntest to just filter out
> > such lines... and now most tests are passing again. In a separate
> > change, I guess I'll fix all the tests.
>
> Fix by making them use the parse testing method? It does sound
> like a better approach.

Ben is looking into fixing the tests -- sounds like the problem wasn't
as extensive as he'd been worried it was, and that the changes are
progressing well.

> > Index: subversion/libsvn_client/merge.c
> > ===================================================================
> > --- subversion/libsvn_client/merge.c (revision 24986)
> > +++ subversion/libsvn_client/merge.c (working copy)
> > @@ -1865,8 +1865,6 @@
> > may create holes in range to merge. Loop over the revision
> > ranges we have left to merge, getting an editor for each range,
> > and applying its delta. */
> > - /* ### FIXME: Handle notification callbacks for multiple merges into
> > - ### a single versioned resource. */
> > for (i = 0; i < remaining_ranges->nelts; i++)
> > {
> > /* When using this merge range, account for the exclusivity of
> > @@ -1875,6 +1873,14 @@
> > svn_merge_range_t *r = APR_ARRAY_IDX(remaining_ranges, i,
> > svn_merge_range_t *);
> >
> > + /* ### TODO(sussman): should this whole for loop be using a
> > + cleared subpool?? */
>
> Yes! I don't want to have to go on another memory saving spree
> like i had to a couple years ago ;->.
>
> > @@ -2211,6 +2215,14 @@
> > svn_merge_range_t *r = APR_ARRAY_IDX(remaining_ranges, i,
> > svn_merge_range_t *);
> >
> > + /* ### TODO(sussman): should this whole for loop be using a
> > + cleared subpool?? */
>
> nod

+1 on using an iter-pool here.