On Sat, Feb 17, 2024 at 12:09 AM Linus Arver via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Linus Arver <linusa@xxxxxxxxxx> > > Currently there are two functions for formatting trailers in > <trailer.h>: > > void format_trailers(const struct process_trailer_options *, > struct list_head *trailers, FILE *outfile); > > void format_trailers_from_commit(struct strbuf *out, const char *msg, > const struct process_trailer_options *opts); > > and although they are similar enough (even taking the same > process_trailer_options struct pointer) they are used quite differently. > One might intuitively think that format_trailers_from_commit() builds on > top of format_trailers(), but this is not the case. Instead > format_trailers_from_commit() calls format_trailer_info() and > format_trailers() is never called in that codepath. > > This is a preparatory refactor to help us deprecate format_trailers() in > favor of format_trailer_info() (at which point we can rename the latter > to the former). When the deprecation is complete, both > format_trailers_from_commit(), and the interpret-trailers builtin will > be able to call into the same helper function (instead of > format_trailers() and format_trailer_info(), respectively). Unifying the > formatters is desirable because it simplifies the API. > > Reorder parameters for format_trailers_from_commit() to prefer > > const struct process_trailer_options *opts > > as the first parameter, because these options are intimately tied to > formatting trailers. And take > > struct strbuf *out > > last, because it's an "out parameter" (something that the caller wants > to use as the output of this function). Here also I think the subject could be more specific like for example: "trailer: reorder format_trailers_from_commit() parameters" > diff --git a/trailer.c b/trailer.c > index d23afa0a65c..5025be97899 100644 > --- a/trailer.c > +++ b/trailer.c > @@ -1083,10 +1083,10 @@ void trailer_info_release(struct trailer_info *info) > free(info->trailers); > } > > -static void format_trailer_info(struct strbuf *out, > +static void format_trailer_info(const struct process_trailer_options *opts, > const struct trailer_info *info, > const char *msg, > - const struct process_trailer_options *opts) > + struct strbuf *out) Ok, so it's not just format_trailers_from_commit() parameters that are reordered, but also format_trailer_info() parameters. It would be nice if the commit message mentioned it.
