Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Emily Shaffer <nasamuffin@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Christian Couder <chriscool@tuxfamily.org>,
	Linus Arver <linusa@google.com>
Subject: [PATCH 00/10] Enrich Trailer API
Date: Wed, 10 Jan 2024 06:51:16 +0000	[thread overview]
Message-ID: <pull.1632.git.1704869487.gitgitgadget@gmail.com> (raw)

This patch series is the first 10 patches of a much larger series I've been
working. The main goal of this series is to enrich the API in trailer.h. The
larger series brings a number of additional code simplifications and
cleanups (exposing and fixing some bugs along the way), and builds on top of
this series. The goal of the larger series is to make the trailer interface
ready for unit testing. By "trailer API" I mean those functions exposed in
trailer.h.

Currently, we have "process_trailers()" in trailer.h which does many
different things (parse command-line arguments, create temporary files, etc)
that are independent of the concept of "trailers", whose interface trailer.h
defines. This makes unit-testing this function difficult. While there is no
technical reason why we couldn't write unit tests for the smaller functions
that are called within process_trailers(), doing so would involve testing
private ("static" in trailer.c) functions instead of the public functions
exposed in trailer.h which authoritatively define the API.

As an alternative to this patch series, we could keep trailer.h intact and
decide to unit-test the existing "trailer_info_get()" function which does
most of the trailer parsing work. However this function wouldn't be easy to
test either, because the resulting data structure merely contains the
unparsed "trailers" lines. So the unit test (if it wants to inspect the
result of parsing these lines) would have to invoke additional parsing
functions itself. And at that point it would not longer be a unit test in
the traditional sense, because it would be invoking multiple functions at
once.

This series breaks up "process_trailers()" into smaller pieces, exposing
many of the parts relevant to trailer-related processing in trailer.h. This
forces us to start writing unit tests for these now public functions, but
that is a good thing because those same unit tests should be easy to write
(due to their small(er) sizes), but also, because those unit tests will now
ensure some degree of stability across new versions of trailer.h (we will
start noticing when the behavior of any of these API functions change).

Another benefit is that more clients (perhaps those outside of Git in the
future) will be able to use the same trailer processing algorithms used by
the interpret-trailers builtin. For example, a web server may want to parse
trailers the same way that interpret-trailers would parse them, without
having to shell out to interpret-trailers. Importing the new (richer)
trailer.h file that this series promotes would help them achieve that goal.
This use case was the original motivation behind my work in this area of
Git.

Thanks to the aggressive refactoring in this series, I've been able to
identify and fix a couple bugs in our existing implementation. Those fixes
build on top of this series but were not included here, in order to keep
this series small.

Linus Arver (10):
  trailer: move process_trailers() to interpret-trailers.c
  trailer: include "trailer" term in API functions
  trailer: unify trailer formatting machinery
  trailer: delete obsolete formatting functions
  sequencer: use the trailer iterator
  trailer: make trailer_info struct private
  trailer: spread usage of "trailer_block" language
  trailer: prepare to move parse_trailers_from_command_line_args() to
    builtin
  trailer: move arg handling to interpret-trailers.c
  trailer: delete obsolete argument handling code from API

 builtin/interpret-trailers.c | 169 ++++++++--
 builtin/shortlog.c           |   7 +-
 pretty.c                     |   2 +-
 ref-filter.c                 |   2 +-
 sequencer.c                  |  35 +--
 trailer.c                    | 579 ++++++++++++++++-------------------
 trailer.h                    | 106 ++++---
 7 files changed, 482 insertions(+), 418 deletions(-)


base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1632%2Flistx%2Ftrailer-api-refactor-part-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1632/listx/trailer-api-refactor-part-1-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1632
-- 
gitgitgadget

             reply	other threads:[~2024-01-10  6:51 UTC|newest]

Thread overview: 202+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-10  6:51 Linus Arver via GitGitGadget [this message]
2024-01-10  6:51 ` [PATCH 01/10] trailer: move process_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-01-18 22:26   ` Junio C Hamano
2024-01-19  0:21     ` Linus Arver
2024-01-10  6:51 ` [PATCH 02/10] trailer: include "trailer" term in API functions Linus Arver via GitGitGadget
2024-01-18 22:28   ` Junio C Hamano
2024-01-19  0:12     ` Linus Arver
2024-01-19  0:15       ` Junio C Hamano
2024-01-10  6:51 ` [PATCH 03/10] trailer: unify trailer formatting machinery Linus Arver via GitGitGadget
2024-01-18 22:56   ` Junio C Hamano
2024-01-19  1:12     ` Linus Arver
2024-01-10  6:51 ` [PATCH 04/10] trailer: delete obsolete formatting functions Linus Arver via GitGitGadget
2024-01-19  0:31   ` Junio C Hamano
2024-01-10  6:51 ` [PATCH 05/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-01-19  0:45   ` Junio C Hamano
2024-01-20 20:04     ` Linus Arver
2024-01-22 23:22   ` Linus Arver
2024-01-10  6:51 ` [PATCH 06/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-01-19  0:58   ` Junio C Hamano
2024-01-25 19:35   ` Josh Steadmon
2024-01-25 20:32     ` Junio C Hamano
2024-01-10  6:51 ` [PATCH 07/10] trailer: spread usage of "trailer_block" language Linus Arver via GitGitGadget
2024-01-19  1:03   ` Junio C Hamano
2024-01-20 20:09     ` Linus Arver
2024-01-10  6:51 ` [PATCH 08/10] trailer: prepare to move parse_trailers_from_command_line_args() to builtin Linus Arver via GitGitGadget
2024-01-25 23:39   ` Josh Steadmon
2024-01-26  0:14     ` Linus Arver
2024-01-10  6:51 ` [PATCH 09/10] trailer: move arg handling to interpret-trailers.c Linus Arver via GitGitGadget
2024-01-19  1:14   ` Junio C Hamano
2024-01-20 20:14     ` Linus Arver
2024-01-10  6:51 ` [PATCH 10/10] trailer: delete obsolete argument handling code from API Linus Arver via GitGitGadget
2024-01-10 19:45 ` [PATCH 00/10] Enrich Trailer API Junio C Hamano
2024-01-13  1:35   ` Linus Arver
2024-01-14 20:05     ` Linus Arver
2024-01-25 23:54 ` Josh Steadmon
2024-01-26 22:38 ` [PATCH v2 " Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 01/10] trailer: prepare to expose functions as part of API Linus Arver via GitGitGadget
2024-01-30  0:44     ` Josh Steadmon
2024-01-30  2:43       ` Linus Arver
2024-01-26 22:38   ` [PATCH v2 02/10] trailer: move interpret_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 03/10] trailer: unify trailer formatting machinery Linus Arver via GitGitGadget
2024-01-30  0:24     ` Josh Steadmon
2024-01-30  2:58       ` Linus Arver
2024-01-26 22:38   ` [PATCH v2 04/10] trailer: delete obsolete formatting functions Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 05/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 06/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 07/10] trailer: spread usage of "trailer_block" language Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 08/10] trailer: prepare to move parse_trailers_from_command_line_args() to builtin Linus Arver via GitGitGadget
2024-01-26 22:38   ` [PATCH v2 09/10] trailer: move arg handling to interpret-trailers.c Linus Arver via GitGitGadget
2024-01-28  5:01     ` Linus Arver
2024-01-28  6:39     ` Linus Arver
2024-01-26 22:38   ` [PATCH v2 10/10] trailer: delete obsolete argument handling code from API Linus Arver via GitGitGadget
2024-01-31  1:22   ` [PATCH v3 00/10] Enrich Trailer API Linus Arver via GitGitGadget
2024-01-31  1:22     ` [PATCH v3 01/10] trailer: prepare to expose functions as part of API Linus Arver via GitGitGadget
2024-01-31  1:22     ` [PATCH v3 02/10] trailer: move interpret_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-01-31 18:54       ` Junio C Hamano
2024-01-31 23:20         ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 03/10] trailer: unify trailer formatting machinery Linus Arver via GitGitGadget
2024-01-31 20:02       ` Josh Steadmon
2024-01-31 23:21         ` Linus Arver
2024-02-01 17:48           ` Junio C Hamano
2024-02-01 18:22             ` Linus Arver
2024-01-31 20:13       ` Junio C Hamano
2024-01-31 22:16         ` Junio C Hamano
2024-02-01  0:46           ` Linus Arver
2024-02-01  1:07             ` Junio C Hamano
2024-02-01 16:41               ` Junio C Hamano
2024-02-01 18:26               ` Linus Arver
2024-02-01 19:21                 ` Junio C Hamano
2024-02-02  7:23                   ` Linus Arver
2024-02-02 17:26                     ` Junio C Hamano
2024-01-31 23:29         ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-02-01 18:06       ` Junio C Hamano
2024-02-01 19:14         ` Linus Arver
2024-02-03  0:39           ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 05/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-02-01 18:49       ` Junio C Hamano
2024-02-03  1:09         ` Linus Arver
2024-02-03  4:43           ` Junio C Hamano
2024-01-31  1:22     ` [PATCH v3 06/10] trailer: spread usage of "trailer_block" language Linus Arver via GitGitGadget
2024-02-01 18:57       ` Junio C Hamano
2024-02-03  1:37         ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 07/10] trailer: prepare to move parse_trailers_from_command_line_args() to builtin Linus Arver via GitGitGadget
2024-02-01 19:06       ` Junio C Hamano
2024-01-31  1:22     ` [PATCH v3 08/10] trailer: move arg handling to interpret-trailers.c Linus Arver via GitGitGadget
2024-02-01 22:23       ` Junio C Hamano
2024-02-03  1:48         ` Linus Arver
2024-02-06  1:01         ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 09/10] trailer: delete obsolete argument handling code from API Linus Arver via GitGitGadget
2024-02-01 22:25       ` Junio C Hamano
2024-02-03  1:40         ` Linus Arver
2024-01-31  1:22     ` [PATCH v3 10/10] trailer: introduce "template" term for readability Linus Arver via GitGitGadget
2024-02-06  5:12     ` [PATCH v4 00/28] Enrich Trailer API Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 01/28] trailer: free trailer_info _after_ all related usage Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 02/28] shortlog: add test for de-duplicating folded trailers Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 03/28] trailer: prepare to expose functions as part of API Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 04/28] trailer: move interpret_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 05/28] trailer: start preparing for formatting unification Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 06/28] trailer_info_get(): reorder parameters Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 07/28] format_trailers(): use strbuf instead of FILE Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 08/28] format_trailer_info(): move "fast path" to caller Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 09/28] format_trailers_from_commit(): indirectly call trailer_info_get() Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 10/28] format_trailer_info(): use trailer_item objects Linus Arver via GitGitGadget
2024-02-09 21:53         ` Junio C Hamano
2024-02-13 16:35           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 11/28] format_trailer_info(): drop redundant unfold_value() Linus Arver via GitGitGadget
2024-02-09 21:54         ` Junio C Hamano
2024-02-06  5:12       ` [PATCH v4 12/28] format_trailer_info(): append newline for non-trailer lines Linus Arver via GitGitGadget
2024-02-09 21:53         ` Junio C Hamano
2024-02-12 23:37         ` Christian Couder
2024-02-13 16:49           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 13/28] trailer: begin formatting unification Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 14/28] format_trailer_info(): teach it about opts->trim_empty Linus Arver via GitGitGadget
2024-02-12 23:38         ` Christian Couder
2024-02-13 17:05           ` Linus Arver
2024-02-13 17:21             ` Christian Couder
2024-02-06  5:12       ` [PATCH v4 15/28] format_trailer_info(): avoid double-printing the separator Linus Arver via GitGitGadget
2024-02-12 23:38         ` Christian Couder
2024-02-13 17:21           ` Linus Arver
2024-02-13 17:25             ` Christian Couder
2024-02-13 19:52               ` Linus Arver
2024-03-15  5:31                 ` Linus Arver
2024-02-13 20:41         ` Kristoffer Haugsbakk
2024-02-06  5:12       ` [PATCH v4 16/28] trailer: finish formatting unification Linus Arver via GitGitGadget
2024-02-09 21:53         ` Junio C Hamano
2024-02-12 23:38         ` Christian Couder
2024-02-13 17:30           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 17/28] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 18/28] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 19/28] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-02-09 21:53         ` Junio C Hamano
2024-02-13 17:36           ` Linus Arver
2024-02-12 23:38         ` Christian Couder
2024-02-13 17:41           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 20/28] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 21/28] trailer: spread usage of "trailer_block" language Linus Arver via GitGitGadget
2024-02-12 23:39         ` Christian Couder
2024-02-13 17:47           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 22/28] trailer: prepare to delete "parse_trailers_from_command_line_args()" Linus Arver via GitGitGadget
2024-02-12 23:39         ` Christian Couder
2024-02-13 17:53           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 23/28] trailer: add new helper functions to API Linus Arver via GitGitGadget
2024-02-12 23:39         ` Christian Couder
2024-02-13 17:57           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 24/28] trailer_add_arg_item(): drop new_trailer_item usage Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 25/28] trailer: deprecate "new_trailer_item" struct from API Linus Arver via GitGitGadget
2024-02-06  5:12       ` [PATCH v4 26/28] trailer: unify "--trailer ..." arg handling Linus Arver via GitGitGadget
2024-02-12 23:39         ` Christian Couder
2024-02-13 18:12           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 27/28] trailer_set_*(): put out parameter at the end Linus Arver via GitGitGadget
2024-02-12 23:39         ` Christian Couder
2024-02-13 18:14           ` Linus Arver
2024-02-06  5:12       ` [PATCH v4 28/28] trailer: introduce "template" term for readability Linus Arver via GitGitGadget
2024-02-12 23:40         ` Christian Couder
2024-02-13 18:20           ` Linus Arver
2024-02-12 23:37       ` [PATCH v4 00/28] Enrich Trailer API Christian Couder
2024-02-13  0:11         ` Junio C Hamano
2024-02-13  6:55           ` Christian Couder
2024-02-13 17:30             ` Junio C Hamano
2024-02-13 20:25               ` Christian Couder
2024-02-16  2:25                 ` Linus Arver
2024-02-13 19:39         ` Linus Arver
2024-02-13 19:57           ` Junio C Hamano
2024-02-13 20:25             ` Kristoffer Haugsbakk
2024-02-13 20:55           ` Christian Couder
2024-02-16  2:17             ` Linus Arver
2024-02-16 23:09       ` [PATCH v5 0/9] " Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 1/9] trailer: free trailer_info _after_ all related usage Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 2/9] shortlog: add test for de-duplicating folded trailers Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 3/9] trailer: prepare to expose functions as part of API Linus Arver via GitGitGadget
2024-02-19 21:31           ` Christian Couder
2024-02-29 22:33             ` Linus Arver
2024-02-29 23:21               ` Junio C Hamano
2024-02-29 23:53                 ` Linus Arver
2024-02-16 23:09         ` [PATCH v5 4/9] trailer: move interpret_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 5/9] trailer: start preparing for formatting unification Linus Arver via GitGitGadget
2024-02-19 21:31           ` Christian Couder
2024-02-29 22:53             ` Linus Arver
2024-02-16 23:09         ` [PATCH v5 6/9] trailer_info_get(): reorder parameters Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 7/9] format_trailers(): use strbuf instead of FILE Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 8/9] format_trailer_info(): move "fast path" to caller Linus Arver via GitGitGadget
2024-02-16 23:09         ` [PATCH v5 9/9] format_trailers_from_commit(): indirectly call trailer_info_get() Linus Arver via GitGitGadget
2024-02-19 21:32           ` Christian Couder
2024-02-29 23:00             ` Linus Arver
2024-02-19 21:40         ` [PATCH v5 0/9] Enrich Trailer API Christian Couder
2024-03-01  0:14         ` [PATCH v6 " Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 1/9] trailer: free trailer_info _after_ all related usage Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 2/9] shortlog: add test for de-duplicating folded trailers Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 3/9] trailer: rename functions to use 'trailer' Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 4/9] trailer: move interpret_trailers() to interpret-trailers.c Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 5/9] trailer: reorder format_trailers_from_commit() parameters Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 6/9] trailer_info_get(): reorder parameters Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 7/9] format_trailers(): use strbuf instead of FILE Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 8/9] format_trailer_info(): move "fast path" to caller Linus Arver via GitGitGadget
2024-03-01  0:14           ` [PATCH v6 9/9] format_trailers_from_commit(): indirectly call trailer_info_get() Linus Arver via GitGitGadget
2024-03-05 18:03           ` [PATCH v6 0/9] Enrich Trailer API Junio C Hamano
2024-03-05 19:07             ` Josh Steadmon
2024-03-05 19:41               ` Junio C Hamano
2024-03-06 14:41               ` Christian Couder
2024-03-06 16:59                 ` Junio C Hamano
2024-03-06 17:09                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1632.git.1704869487.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=linusa@google.com \
    --cc=nasamuffin@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).