You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2011/10/20 15:18:02 UTC
svn commit: r1186791 - /subversion/branches/1.7.x/STATUS
Author: ivan
Date: Thu Oct 20 13:18:02 2011
New Revision: 1186791
URL: http://svn.apache.org/viewvc?rev=1186791&view=rev
Log:
* STATUS: Cast a -0 vote for the r1185746 change.
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=1186791&r1=1186790&r2=1186791&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Thu Oct 20 13:18:02 2011
@@ -28,6 +28,8 @@ Candidate changes:
Fix up some erroneous "Could not frob some targets because..." warnings.
Votes:
+1: stsp, rhuijben
+ -0: ivan (breaking ABI even for private function is not good thing for
+ patch release)
* r1186422, r1186434, r1186732, r1186755
Fix issue 4033, upgrade with deleted=TRUE replaced nodes.
Re: svn commit: r1186791 - /subversion/branches/1.7.x/STATUS
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Oct 20, 2011 at 17:26, Hyrum K Wright <hy...@wandisco.com> wrote:
> On Thu, Oct 20, 2011 at 8:18 AM, <iv...@apache.org> wrote:
>> Author: ivan
>> Date: Thu Oct 20 13:18:02 2011
>> New Revision: 1186791
>>
>> URL: http://svn.apache.org/viewvc?rev=1186791&view=rev
>> Log:
>> * STATUS: Cast a -0 vote for the r1185746 change.
>>
>> 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=1186791&r1=1186790&r2=1186791&view=diff
>> ==============================================================================
>> --- subversion/branches/1.7.x/STATUS (original)
>> +++ subversion/branches/1.7.x/STATUS Thu Oct 20 13:18:02 2011
>> @@ -28,6 +28,8 @@ Candidate changes:
>> Fix up some erroneous "Could not frob some targets because..." warnings.
>> Votes:
>> +1: stsp, rhuijben
>> + -0: ivan (breaking ABI even for private function is not good thing for
>> + patch release)
>
> I don't understand how this is breaking ABI. The only function
> signature that changes in r1185746 is this:
>
[..]
>
> That's a binary-private function, completely within the command line
> client. It doesn't cross any library boundaries. We make these types
> of changes frequently in patch releases, which makes me a little
> confused as to the reasoning behind your -0.
>
Oops, my fault. I was thinking this function cross library boundary.
I'm going to remove my -0 vote. Thanks!
--
Ivan Zhakov
Re: svn commit: r1186791 - /subversion/branches/1.7.x/STATUS
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Oct 20, 2011 at 17:26, Hyrum K Wright <hy...@wandisco.com> wrote:
> On Thu, Oct 20, 2011 at 8:18 AM, <iv...@apache.org> wrote:
>> Author: ivan
>> Date: Thu Oct 20 13:18:02 2011
>> New Revision: 1186791
>>
>> URL: http://svn.apache.org/viewvc?rev=1186791&view=rev
>> Log:
>> * STATUS: Cast a -0 vote for the r1185746 change.
>>
>> 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=1186791&r1=1186790&r2=1186791&view=diff
>> ==============================================================================
>> --- subversion/branches/1.7.x/STATUS (original)
>> +++ subversion/branches/1.7.x/STATUS Thu Oct 20 13:18:02 2011
>> @@ -28,6 +28,8 @@ Candidate changes:
>> Fix up some erroneous "Could not frob some targets because..." warnings.
>> Votes:
>> +1: stsp, rhuijben
>> + -0: ivan (breaking ABI even for private function is not good thing for
>> + patch release)
>
> I don't understand how this is breaking ABI. The only function
> signature that changes in r1185746 is this:
>
[..]
>
> That's a binary-private function, completely within the command line
> client. It doesn't cross any library boundaries. We make these types
> of changes frequently in patch releases, which makes me a little
> confused as to the reasoning behind your -0.
>
Oops, my fault. I was thinking this function cross library boundary.
I'm going to remove my -0 vote. Thanks!
--
Ivan Zhakov
Re: svn commit: r1186791 - /subversion/branches/1.7.x/STATUS
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Oct 20, 2011 at 8:18 AM, <iv...@apache.org> wrote:
> Author: ivan
> Date: Thu Oct 20 13:18:02 2011
> New Revision: 1186791
>
> URL: http://svn.apache.org/viewvc?rev=1186791&view=rev
> Log:
> * STATUS: Cast a -0 vote for the r1185746 change.
>
> 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=1186791&r1=1186790&r2=1186791&view=diff
> ==============================================================================
> --- subversion/branches/1.7.x/STATUS (original)
> +++ subversion/branches/1.7.x/STATUS Thu Oct 20 13:18:02 2011
> @@ -28,6 +28,8 @@ Candidate changes:
> Fix up some erroneous "Could not frob some targets because..." warnings.
> Votes:
> +1: stsp, rhuijben
> + -0: ivan (breaking ABI even for private function is not good thing for
> + patch release)
I don't understand how this is breaking ABI. The only function
signature that changes in r1185746 is this:
[[[
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1185745)
+++ subversion/svn/cl.h (revision 1185746)
@@ -294,15 +294,14 @@
* invoked on an unversioned, nonexistent, or otherwise innocuously
* errorful resource. Meant to be wrapped with SVN_ERR().
*
- * If ERR is null, return SVN_NO_ERROR, setting *SUCCESS to TRUE
- * if SUCCESS is not NULL.
+ * If ERR is null, return SVN_NO_ERROR.
*
* Else if ERR->apr_err is one of the error codes supplied in varargs,
* then handle ERR as a warning (unless QUIET is true), clear ERR, and
- * return SVN_NO_ERROR, setting *SUCCESS to FALSE if SUCCESS is not
- * NULL.
+ * return SVN_NO_ERROR, and push the value of ERR->apr_err into the
+ * ERRORS_SEEN array, if ERRORS_SEEN is not NULL.
*
- * Else return ERR, setting *SUCCESS to FALSE if SUCCESS is not NULL.
+ * Else return ERR.
*
* Typically, error codes like SVN_ERR_UNVERSIONED_RESOURCE,
* SVN_ERR_ENTRY_NOT_FOUND, etc, are supplied in varargs. Don't
@@ -310,7 +309,7 @@
*/
svn_error_t *
svn_cl__try(svn_error_t *err,
- svn_boolean_t *success,
+ apr_array_header_t *errors_seen,
svn_boolean_t quiet,
...);
]]]
That's a binary-private function, completely within the command line
client. It doesn't cross any library boundaries. We make these types
of changes frequently in patch releases, which makes me a little
confused as to the reasoning behind your -0.
Full disclosure: although the code is a bit too verbose for my liking,
I was going to +1 the change before your -0 came in. :)
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1186791 - /subversion/branches/1.7.x/STATUS
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Oct 20, 2011 at 8:18 AM, <iv...@apache.org> wrote:
> Author: ivan
> Date: Thu Oct 20 13:18:02 2011
> New Revision: 1186791
>
> URL: http://svn.apache.org/viewvc?rev=1186791&view=rev
> Log:
> * STATUS: Cast a -0 vote for the r1185746 change.
>
> 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=1186791&r1=1186790&r2=1186791&view=diff
> ==============================================================================
> --- subversion/branches/1.7.x/STATUS (original)
> +++ subversion/branches/1.7.x/STATUS Thu Oct 20 13:18:02 2011
> @@ -28,6 +28,8 @@ Candidate changes:
> Fix up some erroneous "Could not frob some targets because..." warnings.
> Votes:
> +1: stsp, rhuijben
> + -0: ivan (breaking ABI even for private function is not good thing for
> + patch release)
I don't understand how this is breaking ABI. The only function
signature that changes in r1185746 is this:
[[[
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1185745)
+++ subversion/svn/cl.h (revision 1185746)
@@ -294,15 +294,14 @@
* invoked on an unversioned, nonexistent, or otherwise innocuously
* errorful resource. Meant to be wrapped with SVN_ERR().
*
- * If ERR is null, return SVN_NO_ERROR, setting *SUCCESS to TRUE
- * if SUCCESS is not NULL.
+ * If ERR is null, return SVN_NO_ERROR.
*
* Else if ERR->apr_err is one of the error codes supplied in varargs,
* then handle ERR as a warning (unless QUIET is true), clear ERR, and
- * return SVN_NO_ERROR, setting *SUCCESS to FALSE if SUCCESS is not
- * NULL.
+ * return SVN_NO_ERROR, and push the value of ERR->apr_err into the
+ * ERRORS_SEEN array, if ERRORS_SEEN is not NULL.
*
- * Else return ERR, setting *SUCCESS to FALSE if SUCCESS is not NULL.
+ * Else return ERR.
*
* Typically, error codes like SVN_ERR_UNVERSIONED_RESOURCE,
* SVN_ERR_ENTRY_NOT_FOUND, etc, are supplied in varargs. Don't
@@ -310,7 +309,7 @@
*/
svn_error_t *
svn_cl__try(svn_error_t *err,
- svn_boolean_t *success,
+ apr_array_header_t *errors_seen,
svn_boolean_t quiet,
...);
]]]
That's a binary-private function, completely within the command line
client. It doesn't cross any library boundaries. We make these types
of changes frequently in patch releases, which makes me a little
confused as to the reasoning behind your -0.
Full disclosure: although the code is a bit too verbose for my liking,
I was going to +1 the change before your -0 came in. :)
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/