Re: [PATCH v3 1/3] builtin/commit.c: refactor --trailer logic
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 30, 2024, 16:38 UTC
- Message-ID
- <xmqq7cgest5a.fsf@gitster.g>
- In-Reply-To
- <ZjCHiPS8N-eSBrQV@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 5 quoted lines
>> +int amend_file_with_trailers(const char *path, struct strvec const* trailer_args) {
>
> I would have called this `amend_trailers_to_file()`, which feels a bit
> easier to understand. But I don't mind this much, your version should be
> okay, too.I had the same reaction, but we are editing trailers in the file (without touching other things in the same file), so I would have suggested "in" instead of "to" here.
I agree with everything else you said you your review, and I do not think I have anything to add to this step.
> Arguably you don't have to include "strvec.h" here, but can instead add > a simple forward declaration `struct strvec`.
Sensible.
Thanks.