> -----Original Message-----
> From: lieven.govaerts_at_gmail.com [mailto:lieven.govaerts_at_gmail.com] On
> Behalf Of Lieven Govaerts
> Sent: donderdag 21 juli 2011 15:29
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS
>
> Bert,
>
> On Thu, Jul 21, 2011 at 2:04 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
> >
> >
> >> -----Original Message-----
> >> From: lgo_at_apache.org [mailto:lgo_at_apache.org]
> >> Sent: donderdag 21 juli 2011 13:08
> >> To: commits_at_subversion.apache.org
> >> Subject: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS
> >>
> >> Author: lgo
> >> Date: Thu Jul 21 11:07:44 2011
> >> New Revision: 1149116
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1149116&view=rev
> >> Log:
> >> * STATUS: Reviewed and tested issue3888 branch: +1 -> approved.
> >
> > Did you review the branch (two trivial changes) or the behavior change?
:)
> >
>
> I know how this code should behave, as I:
> 1. already had a first try on solving this issue:
> http://svn.apache.org/viewvc?view=revision&revision=1081141
> 2. Discussed with Ivan some of the drawbacks of my implementation:
> http://svn.haxx.se/dev/archive-2011-03/0440.shtml
> 3. Looked at and reviewed some of Greg's changes:
> http://svn.haxx.se/dev/archive-2011-06/0503.shtml
>
> This morning I spent two hours reviewing the implementation, and
> testing with large checkouts and updates that the mechanism is enabled
> and solves the memory leak problem, both with trunk and 1.7.x on Mac
> OS X. For me that gave me enough confidence to add my +1 to the
> backport votes.
>
> Does that mean this code is bug-free? Probably not, but there is nu
> such guarantee for any of the other backported changes either.
>
> Since you are openly doubting my integrity in testing and approving
> changes for backport, I propose you review the code changes yourself
> and add your own +1 or -1 vote.
I think a summary of this would have been a better log message then the
current one, which just says that you reviewed the branch, instead of the
fix.
(That is why I added the ':)' at the end, but I think I should have said it
in a different way)
I intend to look at this patch in more detail later, assuming that this has
had enough review for now. I'm trying to look at a few other open issues
that missed review before.
Bert
Received on 2011-07-21 16:00:15 CEST