Christian Couder <christian.couder@xxxxxxxxx> writes: > 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" Applied, thanks. >> 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. Agreed. Will do.
