> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
> Sent: dinsdag 9 juli 2013 15:27
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org; commits_at_subversion.apache.org
> Subject: Re: svn commit: r1501049 - in /subversion/trunk/subversion:
> include/svn_error_codes.h libsvn_ra_serf/util_error.c
>
> Can you please give me a little credit? The problem here is not that
> 120171 doesn't get stringified in maintainer mode.
>
> The problem here is that it is a number which _does not have_ a symbolic
> name in APR's or Subversion's API, so if an API user wants to code
> against it they need to either hard-code the number or #include <serf.h>
> in their builds --- and last I checked, we didn't require API users to
> do either of those. Compare this to sqlite: we wrap all SQLite error
> integers to SVN_ERR_SQLITE_ERROR, except one or two which we decided
> to
> be important so we get them their own svn error numbers.
>
> And it's not hiding any codes, they are still in the chain as
> err->child->apr_err (or svn_error_find_cause() if you prefer that).
Users don't care about the enum mapping. All they see is an error like:
svn: E230003: Unable to connect to a repository at URL
'https://svn.apache.org:80'
svn: E230003: Error running context: An error occurred during SSL
communication
svn: E120171: APR does not understand this error code
^^^ I don't think this new line you added in r1501049 doesn't help any user.
The tens of thousands of Windows defined error codes are not mapped by
Subversion or apr either, but they are certainly useful for applications. We
never documented that all of them must be mapped as enum value. apr_status
is an int for a reason.
And at least one other public use of the currently undefined values is that
applications can use them themselves and are free to return them from
callbacks for specific error cases.
Do you want to map all of these to SVN_ERR_DOESNT_UNDERSTAND_THIS too?
We shouldn't be in the business of hiding things for bindings that currently
use this behavior.
Are you a binding developer or binding user?
Why do you want to break existing code in TortoiseSVN, AnkhSVN, Subclipse,
etc. etc.?
End users want clear *messages*
Applications want clear *codes*
Your patch breaks both cases, by making the message harder to understand and
hiding the code further in the error path.
Bert
Received on 2013-07-09 15:36:02 CEST