You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2011/07/21 14:04:04 UTC

RE: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS


> -----Original Message-----
> From: lgo@apache.org [mailto:lgo@apache.org]
> Sent: donderdag 21 juli 2011 13:08
> To: commits@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? :)

	Bert 



RE: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: lieven.govaerts@gmail.com [mailto:lieven.govaerts@gmail.com] On
> Behalf Of Lieven Govaerts
> Sent: donderdag 21 juli 2011 15:29
> To: Bert Huijben
> Cc: dev@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 <be...@qqmail.nl> wrote:
> >
> >
> >> -----Original Message-----
> >> From: lgo@apache.org [mailto:lgo@apache.org]
> >> Sent: donderdag 21 juli 2011 13:08
> >> To: commits@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


Re: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS

Posted by Lieven Govaerts <lg...@mobsol.be>.
Bert,

On Thu, Jul 21, 2011 at 2:04 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: lgo@apache.org [mailto:lgo@apache.org]
>> Sent: donderdag 21 juli 2011 13:08
>> To: commits@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.

Lieven

Re: svn commit: r1149116 - /subversion/branches/1.7.x/STATUS

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Jul 21, 2011 at 08:04, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: lgo@apache.org [mailto:lgo@apache.org]
>> Sent: donderdag 21 juli 2011 13:08
>> To: commits@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? :)

lgo said: "tested"

Is there something more needed?

Cheers,
-g