| Message ID | pull.1696.v2.git.1713504153.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Make trailer_info struct private (plus sequencer cleanup) | expand |
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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. As we haven't seen any interest or reviews to this series over its two iterations, I took a look myself and it looked mostly OK to me. So, I'll mark the topic for 'next' unless somebody objects (I really was hoping that Christian would utter something on the topic as it has been his area all along), but given that we'd be in pre-release freeze for one more week, there is no need to rush. Thanks.
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/pull.1694.git.1710485706.gitgitgadget@gmail.com/ [2] https://lore.kernel.org/git/pull.1632.v4.git.1707196348.gitgitgadget@gmail.com/ [3] https://lore.kernel.org/git/CAP8UFD08F0V13X0+CJ1uhMPzPWVMs2okGVMJch0DkQg5M3BWLA@mail.gmail.com/ [4] https://lore.kernel.org/git/CAP8UFD1twELGKvvesxgCrZrypKZpgSt04ira3mvurG1UbpDfxQ@mail.gmail.com/ 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 <linusa@google.com> + ## 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