ok, will drop the patch due to bad design.
On Thu, Nov 20, 2014 at 10:36 AM, Ronnie Sahlberg <sahlberg@xxxxxxxxxx> wrote:
> On Thu, Nov 20, 2014 at 10:35 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
>> Stefan Beller wrote:
>>
>>> If we don't pass in the error string buffer, we skip over all
>>> parts dealing with preparing error messages.
>>
>> Please no.
>>
>> We tried this with the ref transaction code. When someone wants
>> to silence the message, it is cheap enough to do
>>
>> struct strbuf ignore = STRBUF_INIT;
>>
>> if (thing_that_can_fail_in_an_ignorable_way(..., &ignore)) {
>> ... handle the failure ...
>> }
>>
>> The extra lines of code make it obvious that the error message is
>> being dropped, which is a very good thing. The extra work to format a
>> message in the error case is not so bad and can be mitigated if the
>> error is a common normal case by passing a flag to not consider it an
>> error.
>>
>> Silently losing good diagnostic messages when err == NULL would have
>> the opposite effect: when there isn't a spare strbuf to put errors in
>> around, it would be tempting for people coding in a hurry to just pass
>> NULL, and to readers it would look at first glance like "oh, an
>> optional paramter was not passed and we are getting the good default
>> behavior".
>>
>> This is not a theoretical concern --- it actually happened.
>>
>
> Fair enough.
> Un-LGTM my message above.
>
>
>
>> My two cents,
>> Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html