You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by lg...@apache.org on 2011/07/21 13:07:45 UTC

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.

Modified:
    subversion/branches/1.7.x/STATUS

Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1149116&r1=1149115&r2=1149116&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Thu Jul 21 11:07:44 2011
@@ -71,14 +71,6 @@ Candidate changes:
      +1: philip
      -1: gstein (I see no explanation for why "serf is not ready")
 
- * 1.7.x-issue3888 branch
-   Fix issue 3888 for 1.7.x
-   Justification:
-     For very large checkouts/updates/switches, ra_serf may consume
-     unbounded memory. That is not good.
-   Votes:
-     +1: gstein, jerenkrantz
-
  * r1147540, r1147541
    Remove unused variables in build system.
    Justification:
@@ -113,3 +105,11 @@ Approved changes:
      Avoid calling into a library incompatible with the one compiled against.
    Votes:
      +1: danielsh, gstein, arfrever
+
+ * 1.7.x-issue3888 branch
+   Fix issue 3888 for 1.7.x
+   Justification:
+     For very large checkouts/updates/switches, ra_serf may consume
+     unbounded memory. That is not good.
+   Votes:
+     +1: gstein, jerenkrantz, lgo



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

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

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

> -----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