Re: svn commit: r1499315 - in /subversion/branches/verify-keep-going/subversion: libsvn_repos/dump.c svnadmin/svnadmin.c

Branko Čibej wrote on Wed, Jul 03, 2013 at 13:48:57 +0200:
> On 03.07.2013 13:37, Daniel Shahaf wrote:
> > prabhugs_at_apache.org wrote on Wed, Jul 03, 2013 at 10:49:02 -0000:
> >> + notify_failure->warning_str = apr_psprintf(pool,
> >> + _("E%06d: %s"),
> > Would it make sense to push the E%06d: part down to the implementation
> > of svn_err_best_message()?
> >
> > I.e., if the information is useful for this caller, presumably it'll be
> > useful for others too?
>
> Error message formatting should not be the domain of libsvn_subr.
> Different API consumers may want to format the error codes differently,
> and it's a bad idea to make them parse it out.

This is a #svn_repos_notify_failure notification, and the API does not
define the value of ->warning_str in that case. It seems to me we
should leave it unset; it suffices to pass the error in the ->err member
for the API caller to do with as it pleases.

Good catch.
Received on 2013-07-03 13:53:31 CEST