Re: [PATCH v4 19/28] trailer: make trailer_info struct private

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Couder <christian.couder@xxxxxxxxx> writes:

> On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
>
>> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
>> index 11f4ce9e4a2..6bf8cec005a 100644
>> --- a/builtin/interpret-trailers.c
>> +++ b/builtin/interpret-trailers.c
>> @@ -141,7 +141,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>>         LIST_HEAD(head);
>>         struct strbuf sb = STRBUF_INIT;
>>         struct strbuf trailer_block = STRBUF_INIT;
>> -       struct trailer_info info;
>> +       struct trailer_info *info;
>>         FILE *outfile = stdout;
>>
>>         trailer_config_init();
>> @@ -151,13 +151,13 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>>         if (opts->in_place)
>>                 outfile = create_in_place_tempfile(file);
>>
>> -       parse_trailers(opts, &info, sb.buf, &head);
>> +       info = parse_trailers(opts, sb.buf, &head);
>
> I think this patch might be doing too much at once and could have been
> split into 3 or more patches to make reviews easier.

Ack, will do.

> For example the first patch could introduce trailer_info_new() and
> make interpret_trailers() use it. Then the second patch could modify
> parse_trailers() so that it returns a 'struct trailer_info *'. Then
> the third patch could make the trailer_info struct private.

Thanks, I will try to use this breakdown in the next reroll.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux