Re: [PATCH] Simplify reporting of API violations of the conflict resolver callback.

[Karl Fogel <kf...@red-bean.com>]

Julian Foad <ju...@btopenworld.com> writes:
> If the conflict resolver callback violates its API (e.g. returns NULL or
> doesn't generate a result file when it says it has done), we presently
> generate a custom error message:
>
>>   if (result == NULL)
>>     return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
>>                             NULL, _("Conflict callback violated API:"
>>                                     " returned no results"));
>
> This happens in 5 places and some are longer than the quoted example. My
> instinct is to simplify by using just a SVN_ERR_ASSERT statement, like
> this:
>
>>   SVN_ERR_ASSERT(result);
>
> See the attached patch.
>
> Should I do this?
>
> My reasons: I think it makes the normal code path easier to read. I
> don't think we have custom error messages for other API violation error
> returns. It gets rid of some messages that are meaningful only to
> programmers but of limited use to programmers, and relieves the
> translators of translating them. I can't think of strong
> counter-arguments but I'm wary that there may be some.

With conflict callbacks, it may be particularly easy to write a buggy
callback, and the situation-specific error messages we give right now
can help people figure out what they did wrong.

I have no evidence that the error message are or aren't helpful to
people, but my instinct is to keep them.  Wish I could justify that with
something stronger than "Let's not lose information" :-).

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Simplify reporting of API violations of the conflict resolver callback.

[Julian Foad <ju...@btopenworld.com>]

On Mon, 2008-07-14 at 19:05 -0400, Karl Fogel wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> > Should I do this?
> >
> > My reasons: I think it makes the normal code path easier to read. I
> > don't think we have custom error messages for other API violation error
> > returns. It gets rid of some messages that are meaningful only to
> > programmers but of limited use to programmers, and relieves the
> > translators of translating them. I can't think of strong
> > counter-arguments but I'm wary that there may be some.
> 
> With conflict callbacks, it may be particularly easy to write a buggy
> callback, and the situation-specific error messages we give right now
> can help people figure out what they did wrong.
> 
> I have no evidence that the error message are or aren't helpful to
> people, but my instinct is to keep them.  Wish I could justify that with
> something stronger than "Let's not lose information" :-).

Oh well... I suspected some people would feel like that. To me it gets
in the way, just a little bit, of understanding the code, which is
important, but I guess there are other ways to tackle that.

- Juloi




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org