On Sun, Apr 3, 2016 at 12:45 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Sat, Apr 02, 2016 at 07:16:14PM -0400, santiago@xxxxxxx wrote:
>> - len = parse_signature(buf, size);
>> -
>> - if (size == len) {
>> - if (flags & GPG_VERIFY_VERBOSE)
>> - write_in_full(1, buf, len);
>> - return error("no signature found");
>> - }
>> [...]
>> + payload_size = parse_signature(buf, size);
>> +
>> + if (size == payload_size) {
>> + write_in_full(1, buf, payload_size);
>> + return error("No PGP signature found in this tag!");
>> + }
>
> I'm happy to see the more readable variable name here. I wonder if we
> should leave the error message as-is, though, as this is just supposed
> to be about code movement (and if we are changing it, it should adhere
> to our usual style of not starting with a capital letter, and not ending
> in punctuation).
Agreed it would be nice for this patch to be just code movement since
it's difficult for a reviewer to spot actual changes. A pure code
movement patch was suggested by [1], but perhaps it should also have
explained the reason ("code changes are difficult to spot in
movement").
Such changes could be done as preparatory or follow-on patches.
Alternately, since these are such minor changes, it might also be okay
just to mention them in the commit message (as the function rename is
already mentioned).
[1]: http://article.gmane.org/gmane.comp.version-control.git/289847
--
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