"Linus Arver via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> diff --git a/trailer.h b/trailer.h
> index 1644cd05f60..b3e4a5e127d 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -81,15 +81,29 @@ struct process_trailer_options {
>
> #define PROCESS_TRAILER_OPTIONS_INIT {0}
>
> -void process_trailers(const char *file,
> - const struct process_trailer_options *opts,
> - struct list_head *new_trailer_head);
> +void parse_trailers_from_config(struct list_head *config_head);
> +
> +void parse_trailers_from_command_line_args(struct list_head *arg_head,
> + struct list_head *new_trailer_head);
> +
> +void process_trailers_lists(struct list_head *head,
> + struct list_head *arg_head);
> +
> +void parse_trailers(struct trailer_info *info,
> + const char *str,
> + struct list_head *head,
> + const struct process_trailer_options *opts);
OK.
> +void ensure_configured(void);
> +void print_all(FILE *outfile, struct list_head *head,
> + const struct process_trailer_options *opts);
> +void free_all(struct list_head *head);
All of these names are way overly generic to live in the global
namespace, no? Granted, they may only be visible to folks who
include <trailer.h>, but sooner or later other subsystems would find
need to provide library-ish functions that allows their callers to
do these things to their subsystem: make sure the subsystem has been
initialized, print info on all "things" on a list that the subsystem
works on, and free all "things" on such a list.
It is sad to have a function that takes "struct list_head *" as its
parameter without having any comment on what the elements on that
list are about. free_trailer_items() might be a more appropriate
name (or free_all_trailer_items(), if we foresee a need to just free
the first item on the list and give such a helper
free_trailer_item(), in which case, the distinction between "item"
vs "items" might become too subtle.
I am assuming (and did not verify more than seeing the "git show"
output with "--color-moved") that other changes in this step are all
pure moves? If so, other than need to be careful about naming when
making things from static to extern pointed out above, this step
looks OK to me.
Thanks.
