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.