Josh Steadmon <steadmon@xxxxxxxxxx> writes: > On 2024.01.26 22:38, Linus Arver via GitGitGadget wrote: >> From: Linus Arver <linusa@xxxxxxxxxx> >> >> Currently have two functions for formatting trailers exposed in >> trailer.h: >> >> void format_trailers(FILE *outfile, struct list_head *head, >> const struct process_trailer_options *opts); >> >> void format_trailers_from_commit(struct strbuf *out, const char *msg, >> const struct process_trailer_options *opts); >> >> and previously these functions, although similar enough (even taking the >> same process_trailer_options struct pointer), did not build on each >> other. >> >> Make format_trailers_from_commit() rely on format_trailers(). Teach >> format_trailers() to process trailers with the additional >> process_trailer_options fields like opts->key_only which is only used by >> format_trailers_from_commit() and not builtin/interpret-trailers.c. >> While we're at it, reorder parameters to put the trailer processing >> options first, and the out parameter (strbuf we write into) at the end. >> >> This unification will allow us to delete the format_trailer_info() and >> print_tok_val() functions in the next patch. They are not deleted here >> in order to keep the diff small. > > Unfortunately this breaks the build: > > trailer.c:1145:13: error: ‘format_trailer_info’ defined but not used [-Werror=unused-function] > > and > > trailer.c:147:13: error: ‘print_tok_val’ defined but not used [-Werror=unused-function] > > While separating this patch from the deletion does make it easier to > review, it may make bisection more difficult. FTR I've tried a preview version of squashing the deletion into this patch with "/preview" in GitGitGadget, and it generated a clean-enough diff where the deletions weren't intermixed with additions (maybe it uses the patience diff algorithm). But I didn't squash them for v2 because I was concerned about the range diff becoming even more difficult to read for reviewers. I'm OK with squashing them for v3, but I'm also not sure that's necessary. For example, during bisection you could use DEVOPTS=no-error (or similar) in config.mak to skip over harmless errors like "unused-function". Personally I'd prefer to keep the patches separate because they started separately on the list. Ultimately, I don't have a strong opinion on this. Maybe Junio or someone else can cast the tie-breaking vote? To squash or not to squash? I will take lazy consensus to mean "squash" for v3 if no one has objections. Thanks.
