On Thu, Oct 20, 2011 at 8:18 AM, <i...@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/
