On Mon, Sep 21, 2015 at 3:50 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> On Mon, Sep 21, 2015 at 12:46 AM, Matthieu Moy
> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>>
>>> Remove the error reporting variable to make the code easier to port
>>> over to using ref-filter APIs. This variable
>>
>> (it's not just a variable, it's also a message on the output)
>>
>>> is not required as in ref-filter we already check for possible errors
>>> and report them.
>>
>> You should explain more why the check is redundant with the ones in
>> ref-filter. You discussed this with Junio on the last round. The
>> arguments you gave would deserve to appear in the commit message IMHO.
>> Pointing to the exact checks in ref-filter would help.
>>
>> It may make sense to split the commit into several smaller ones, so that
>> you can give a precise commit message for each error reporting you
>> remove (error(_("branch '%s' does not point at a commit") and
>> error(_("some refs could not be read"))).
>>
>
> I could do that, but it would be small changes. But I think it's worth it,
> will wait for other changes if needed and send.
>
But then again, both those are interrelated, the cb.ret value is set at
(error(_("branch '%s' does not point at a commit"), refname) and the second
error is printed on the basis that the first error set's cb.ret value.
So I find it
better to modify and commit message and leave the commits united.
--
Regards,
Karthik Nayak
--
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