Re: [PATCH v2] ssh signing: better error message when key not in agent

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 25, 2023, 17:17 UTC
Message-ID: <xmqqsffyedoh.fsf@gitster.g>
In-Reply-To: <CAEroKagUY5PfuC2CDn=pTJ=brPsjPy6MVz54mH1tvN8E-Pvk7g@mail.gmail.com>

Adam Szkoda <adaszko@gmail.com> writes:

Show 16 quoted lines

>> We do
>> not pass filename yet with this pushl(), because ...
>>
>> > +     if (literal_ssh_key) {
>> > +             strvec_push(&signer.args, "-U");
>> > +     }
>>
>> ... when we give a literal key, we want to insert "-U" in front, and then
>>
>> > +     strvec_push(&signer.args, buffer_file->filename.buf);
>>
>> ... the filename.  Which makes sense.
>
> I'm not sure what you mean in this paragraph.   If there's something
> more that needs to be done, I'd appreciate it if you could rephrase
> it.

"Which makes sense." is the key conclusion. Instead of saying "This part of the patch looks good" without explaining why I say so (it could be that I am saying so without really reading or understanding the changes or thinking the ramifications of the change through), the earlier parts that lead to the conclusion is a way to give weight to the conclusion.

In other words, it is meant to show that the reviewer did read the patch well enough to understand the reasoning behind it.

Thanks.