Subject: [PATCH v2 0/8] Make trailer_info struct private (plus sequencer cleanup)

From: "Linus Arver via GitGitGadget" <[email-masked]>
Date: Fri, 19 Apr 2024 05:22:25 +0000

NOTE: This series is based on the la/format-trailer-info topic branch (see
its discussion at [1]).

This series is based on the initial series [2], notably the v4 version of
patches 17-20 as suggested by Christian [3]. This version addresses the
review comments for those patches, namely the splitting up of Patch 19 there
into 3 separate patches [4] (as Patches 05-07 here) .

The central idea is to make the trailer_info struct private (that is, move
its definition from trailer.h to trailer.c) --- aka the "pimpl" idiom. See
the detailed commit message for Patch 07 for the motivation behind the
change.

Patch 04 makes sequencer.c a well-behaved trailer API consumer, by making
use of the trailer iterator. Patch 03 prepares us for Patch 04. Patch 08
slightly reduces the weight of the API by removing (from the API surface) an
unused function.

Notable changes in v2
=====================

 * Add unit tests at the beginning of the series (Patches 01 and 02) and use
   it to verify that the other edge cases remain unchanged when we add the
   "raw" member (Patch 03)

[1]
https://lore.kernel.org/git/[email-masked]/
[2]
https://lore.kernel.org/git/[email-masked]/
[3]
https://lore.kernel.org/git/[email-masked]/
[4]
https://lore.kernel.org/git/[email-masked]/

Linus Arver (8):
  Makefile: sort UNIT_TEST_PROGRAMS
  trailer: add unit tests for trailer iterator
  trailer: teach iterator about non-trailer lines
  sequencer: use the trailer iterator
  interpret-trailers: access trailer_info with new helpers
  trailer: make parse_trailers() return trailer_info pointer
  trailer: make trailer_info struct private
  trailer: retire trailer_info_get() from API

 Makefile                     |   5 +-
 builtin/interpret-trailers.c |  12 +--
 sequencer.c                  |  27 +++---
 t/unit-tests/t-trailer.c     | 181 +++++++++++++++++++++++++++++++++++
 trailer.c                    | 161 +++++++++++++++++++------------
 trailer.h                    |  46 ++++-----
 6 files changed, 321 insertions(+), 111 deletions(-)
 create mode 100644 t/unit-tests/t-trailer.c

base-commit: 3452d173241c8b87ecdd67f91f594cb14327e394
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1696%2Flistx%2Ftrailer-api-part-3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1696/listx/trailer-api-part-3-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1696

Range-diff vs v1:

 -:  ----------- > 1:  b6a1304f8ae Makefile: sort UNIT_TEST_PROGRAMS
 -:  ----------- > 2:  e1fa05143ac trailer: add unit tests for trailer iterator
 1:  32ad0397737 ! 3:  5520a98e296 trailer: teach iterator about non-trailer lines
     @@ Commit message
      
          Signed-off-by: Linus Arver <[email-masked]>
      
     + ## t/unit-tests/t-trailer.c ##
     +@@ t/unit-tests/t-trailer.c: static void run_t_trailer_iterator(void)
     + 			"not a trailer line\n"
     + 			"not a trailer line\n"
     + 			"Signed-off-by: x\n",
     +-			1
     ++			/*
     ++			 * Even though there is only really 1 real "trailer"
     ++			 * (Signed-off-by), we still have 4 trailer objects
     ++			 * because we still want to iterate through the entire
     ++			 * block.
     ++			 */
     ++			4
     + 		},
     + 		{
     + 			"with non-trailer lines (one too many) in trailer block",
     +
       ## trailer.c ##
      @@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
       
 2:  dc873c3b820 = 4:  84897cf5c83 sequencer: use the trailer iterator
 3:  872e67286c8 = 5:  e961d49cd40 interpret-trailers: access trailer_info with new helpers
 4:  c55ae2cbda9 = 6:  093f68f3658 trailer: make parse_trailers() return trailer_info pointer
 5:  cf59dee5064 = 7:  0e9ae049b88 trailer: make trailer_info struct private
 6:  19de7c64171 = 8:  eca77a1a462 trailer: retire trailer_info_get() from API

-- 
gitgitgadget