Re: [PATCH v2 3/6] transport-helper: clarify pushing without refspecs

[Junio C Hamano <gitster@pobox.com>]

John Keeping <john@keeping.me.uk> writes:

>> +	It is recommended that all importers providing the 'import'
>> +	capability use this. It's mandatory for 'export'.
>
> s/It's/It is/

I personally do not care _too_ deeply either way, but I agree our
documentation tends to use the latter more and being consistent
would be good.

>> diff --git a/transport-helper.c b/transport-helper.c
>> index cea787c..4d98567 100644
>> --- a/transport-helper.c
>> +++ b/transport-helper.c
>> @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport,
>>  	struct string_list revlist_args = STRING_LIST_INIT_NODUP;
>>  	struct strbuf buf = STRBUF_INIT;
>>  
>> +	if (!data->refspecs)
>> +		die("remote-helper doesn't support push; refspec needed");
>
> I think the "refspec needed" text is likely to be confusing if an
> end-user ever sees this message.  I'm not sure how we can provide useful
> feedback for both remote helper authors and end-users though.

This "refspecs" only come via the helper and not directly from the
end user, no?

If that is the case, I do not think "confusing" is much of an issue.
Not _("localizing") is also the right thing to do.  We may want to
say "BUG: " at front to clarify it is not the end-user's fault, but
a problem they may want to report.  If we at this point know what
helper attempted export without giving refspecs, it may help to show
it here, so that developers will know with what helper the user
had problems with.