mbox series

[0/6] Make trailer_info struct private (plus sequencer cleanup)

Message ID pull.1696.git.1710570428.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Make trailer_info struct private (plus sequencer cleanup) | expand

Message

Philippe Blain via GitGitGadget March 16, 2024, 6:27 a.m. UTC
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 03-05 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 05 for the motivation behind the
change.

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

[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 (6):
  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

 builtin/interpret-trailers.c |  12 +--
 sequencer.c                  |  27 +++---
 trailer.c                    | 161 ++++++++++++++++++++++-------------
 trailer.h                    |  46 ++++------
 4 files changed, 137 insertions(+), 109 deletions(-)


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

Comments

Junio C Hamano March 16, 2024, 5:06 p.m. UTC | #1
"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]).

Folks, a quick review of the base topic is highly appreciated.  Not
having much review to talk about in [1] makes it a bit premature to
build another series on top of it.

Thanks.
Junio C Hamano March 26, 2024, 10 p.m. UTC | #2
"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 unfortunately depends on another series, which has seen no
reviews after 10 days X-<.  It did not help that this was sent
almost immediately after that unreviewed series that it depends on.

Any takers?  There must be some folks who know the trailer code very
well, no?

Thanks.
Linus Arver April 19, 2024, 5:36 a.m. UTC | #3
Junio C Hamano <gitster@pobox.com> writes:

> "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 unfortunately depends on another series, which has seen no
> reviews after 10 days X-<.  It did not help that this was sent
> almost immediately after that unreviewed series that it depends on.
>
> Any takers?  There must be some folks who know the trailer code very
> well, no?
>

I've added some unit test cases in v2 Patch 02 to make this series a bit
more appealing for reviewers. Cheers.