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

From: Junio C Hamano <[email-masked]>
Date: Wed, 25 Jan 2023 09:17:50 -0800

Adam Szkoda <[email-masked]> writes:

>> 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.