Re: [PATCH v3 2/5] ref-filter: add return value && strbuf to handlers

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2018, 19:24 UTC
Message-ID: <xmqqbmfjvpgm.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <010201623e59448f-2db7a920-fe77-481a-b105-de6bbff253eb-000000@eu-west-1.amazonses.com>

Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> Continue removing any printing from ref-filter formatting logic,
> so that it could be more general.

"more general" sounds overly broad. Is this to avoid calling die() so that the caller can decide what error messages it wants to give, abort operation or not, etc.? From a quick scan of the patch, I have a feeling that "any printing" is not the problem you are solving (calling die() is).

> Change the signature of handlers by adding return value
> and strbuf parameter for errors.

Could you explain what the "return value" means? We can see from the patch that the function signature is being changed by the patch. What needs human explanation is why and what the values mean, e.g. "to allow the caller to notice an error, the function returns 0 upon success and non-0 upon failure, and an error message is appended to the strbuf *err" (don't pay too much attention to "0" or "non-0" in this example, as I do not know what you chose to assign the return values to signal what to the callers; this is merely to illustrate the degree of details I would expect).

Thanks.