Re: [PATCH v2] builtin/push: call set_refspecs after validating remote

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2024, 18:56 UTC
Message-ID: <xmqqsewie6mm.fsf@gitster.g>
In-Reply-To: <xmqq4j8yflrq.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Is it merely "this is a nonsense request and must fail, but we do
> not want to hit BUG in general"?  I think it is the latter, but
> leaving it unsaid is confusing.  How about starting it more like...

A bit of clarification. "it is the latter" -> "that is what you meant". In any earlier draft I had another possibility but I removed it before sending the message.

Show 8 quoted lines

>     When an end-user runs "git push" with an empty string for the
>     remote repository name, e.g.
>
>         $ git push '' main
>
>     "git push" fails with a BUG().  This is because ...
>
> or something.

Another clarificaiton.

    ... with a BUG().

->

    ... with a BUG().  Even though this is a nonsense request that
    we want to fail, we shouldn't hit a BUG().  Instead we want to
    give a sensible error message, e.g., 'bad repository'".

Let's make a habit of not stopping at saying what is bad, and also saying what we want instead explicitly.

Thanks.