| Message ID | 5f64718abfc2e61b4e259de700c137bc817fbb1c.1704869487.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enrich Trailer API | expand |
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Linus Arver <linusa@google.com> > > These functions are exposed to clients and so they should include > "trailer" in their names for easier identification, just like all the > other functions already exposed by trailer.h. Oh, if you were to do this and code movement in two separate patches, doing the rename before the move in the previous step would have made much more sense. If we stopped after 01/10, the tree would have been a very sorry state. If this came first, even if we stop after these renaming of internal functions that are not extern, nobody wil be hurt by these new and improved names.
Junio C Hamano <gitster@pobox.com> writes: > "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Linus Arver <linusa@google.com> >> >> These functions are exposed to clients and so they should include >> "trailer" in their names for easier identification, just like all the >> other functions already exposed by trailer.h. > > Oh, if you were to do this and code movement in two separate > patches, doing the rename before the move in the previous step would > have made much more sense. If we stopped after 01/10, the tree > would have been a very sorry state. If this came first, even if we > stop after these renaming of internal functions that are not extern, > nobody wil be hurt by these new and improved names. Ah very true. I think I was thinking of the series as one atomic thing (all 10 patches or nothing), which is a bad habit I need to break. I'll reorder these two on the next reroll, because principles matter (and it's an easy mechanical change). Thanks.
Linus Arver <linusa@google.com> writes: > Ah very true. I think I was thinking of the series as one atomic thing > (all 10 patches or nothing), which is a bad habit I need to break. I'll > reorder these two on the next reroll, because principles matter (and > it's an easy mechanical change). Thanks. Thanks. I wouldn't have minded if these renames and moves were done in a single step, but because you already have them as separate two steps, let's keep them separate.
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c index 444f8fb70c9..adb74276281 100644 --- a/builtin/interpret-trailers.c +++ b/builtin/interpret-trailers.c @@ -143,7 +143,7 @@ static void interpret_trailers(const char *file, struct trailer_info info; FILE *outfile = stdout; - ensure_configured(); + trailer_config_init(); read_input_file(&sb, file); @@ -169,9 +169,10 @@ static void interpret_trailers(const char *file, process_trailers_lists(&head, &arg_head); } - print_all(outfile, &head, opts); + /* Print trailer block. */ + format_trailers(outfile, &head, opts); - free_all(&head); + free_trailers(&head); trailer_info_release(&info); /* Print the lines after the trailers as is */ diff --git a/trailer.c b/trailer.c index 9d70c9946bd..0ce7e9079ca 100644 --- a/trailer.c +++ b/trailer.c @@ -162,8 +162,8 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val) fprintf(outfile, "%s%c %s\n", tok, separators[0], val); } -void print_all(FILE *outfile, struct list_head *head, - const struct process_trailer_options *opts) +void format_trailers(FILE *outfile, struct list_head *head, + const struct process_trailer_options *opts) { struct list_head *pos; struct trailer_item *item; @@ -588,7 +588,7 @@ static int git_trailer_config(const char *conf_key, const char *value, return 0; } -void ensure_configured(void) +void trailer_config_init(void) { if (configured) return; @@ -1023,7 +1023,7 @@ void parse_trailers(struct trailer_info *info, } } -void free_all(struct list_head *head) +void free_trailers(struct list_head *head) { struct list_head *pos, *p; list_for_each_safe(pos, p, head) { @@ -1041,7 +1041,7 @@ void trailer_info_get(struct trailer_info *info, const char *str, size_t nr = 0, alloc = 0; char **last = NULL; - ensure_configured(); + trailer_config_init(); end_of_log_message = find_end_of_log_message(str, opts->no_divider); trailer_block_start = find_trailer_block_start(str, end_of_log_message); diff --git a/trailer.h b/trailer.h index b3e4a5e127d..0e4f0ece9b3 100644 --- a/trailer.h +++ b/trailer.h @@ -99,11 +99,11 @@ void trailer_info_get(struct trailer_info *info, const char *str, void trailer_info_release(struct trailer_info *info); -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); +void trailer_config_init(void); +void free_trailers(struct list_head *trailers); +void format_trailers(FILE *outfile, struct list_head *head, + const struct process_trailer_options *opts); /* * Format the trailers from the commit msg "msg" into the strbuf "out". * Note two caveats about "opts": @@ -111,7 +111,7 @@ void free_all(struct list_head *head); * - this is primarily a helper for pretty.c, and not * all of the flags are supported. * - * - this differs from process_trailers slightly in that we always format + * - this differs from format_trailers slightly in that we always format * only the trailer block itself, even if the "only_trailers" option is not * set. */