Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Linus Arver <linusa@google.com>
To: Junio C Hamano <gitster@pobox.com>,
	 Linus Arver via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Glen Choo <glencbz@gmail.com>,
	 Christian Couder <chriscool@tuxfamily.org>,
	Phillip Wood <phillip.wood123@gmail.com>,
	 Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH v5 2/3] trailer: find the end of the log message
Date: Thu, 28 Dec 2023 22:42:05 -0800	[thread overview]
Message-ID: <owlyv88hqzlu.fsf@fine.c.googlers.com> (raw)
In-Reply-To: <xmqqr0lpoue3.fsf@gitster.g>


TL;DR: I'm working on a new approach.

Junio C Hamano <gitster@pobox.com> writes:
> Other than that, I didn't find anything quesionable in any of the
> patches in this round.  Looking good.

So actually, I'm now taking a much more aggressive approach to libifying
the trailer subsystem. Instead of incrementally simplifying/improving
things as in this series, I think I need to get to the root problem,
which is that the trailer.h API isn't rich enough to make it pleasant
for clients to use, including our own builtin/interpret-trailers.c
client. That is, the problem we have today is that the trailer subsystem
is not very ergonomic for internal use, much less external use (outside
of Git itself).

As an example, the current API exposes process_trailers() which does a
whole bunch of things that only builtin/interpret-trailers.c cares
about. Multiple other clients of trailer.h exist in our codebase (e.g.,
sequencer.c, pretty.c, ref-filter.c) but none of them use
process_trailers().

One really useful data structure is the trailer_iterator that was
introduced in f0939a0eb1 (trailer: add interface for iterating over
commit trailers, 2020-09-27). The only problem is that it is not generic
enough such that interpret-trailers.c can use it.

My new goal is to introduce a new API in trailer.h so that
interpret-trailers.c and everyone else can start using these new data
structures and associated functions (while preserving the
trailer_iterator interface). So the order of operations should be:

(1) enrich the trailer API (make trailer.h have simpler data structures
    and practical functions that clients can readily use), and
(2) make builtin/interpret-trailers.c, and other clients in the Git
    codebase use this new API.

This way when the unit test framework selection process is finalized we
can

(3) write unit tests for the functions in the (enriched) trailer API,

which is one of the major goals for my efforts around this area.

The work I've started locally for (1) does not depend on this series,
and I think it'll be cleaner (less churn) that way. So, feel free to
drop this series in favor of the forthcoming work described in this
message.

Thanks.

  reply	other threads:[~2023-12-29  6:42 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-05  5:04 [PATCH 0/5] Trailer readability cleanups Linus Arver via GitGitGadget
2023-08-05  5:04 ` [PATCH 1/5] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-08-07 21:16   ` Glen Choo
2023-08-08 12:19     ` Phillip Wood
2023-08-10 23:15       ` Linus Arver
2023-08-10 22:50     ` Linus Arver
2023-08-05  5:04 ` [PATCH 2/5] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-08-07 22:39   ` Glen Choo
2023-08-11  0:41     ` Linus Arver
2023-08-05  5:04 ` [PATCH 3/5] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-08-07 22:51   ` Glen Choo
2023-08-11  0:59     ` Linus Arver
2023-08-11  1:02       ` Linus Arver
2023-08-11 21:11       ` Glen Choo
2023-08-05  5:04 ` [PATCH 4/5] trailer: teach find_patch_start about --no-divider Linus Arver via GitGitGadget
2023-08-07 23:28   ` Glen Choo
2023-08-11  1:25     ` Linus Arver
2023-08-11 20:51       ` Glen Choo
2023-08-05  5:04 ` [PATCH 5/5] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-08-07 23:45   ` Glen Choo
2023-08-11 18:00     ` Linus Arver
2023-09-09  6:16 ` [PATCH v2 0/6] Trailer readability cleanups Linus Arver via GitGitGadget
2023-09-09  6:16   ` [PATCH v2 1/6] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-09-11 17:10     ` Junio C Hamano
2023-09-09  6:16   ` [PATCH v2 2/6] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-09-11 17:10     ` Junio C Hamano
2023-09-09  6:16   ` [PATCH v2 3/6] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-09-09  6:16   ` [PATCH v2 4/6] trailer: teach find_patch_start about --no-divider Linus Arver via GitGitGadget
2023-09-11 17:25     ` Junio C Hamano
2023-09-14  2:19       ` Linus Arver
2023-09-14  3:12         ` Junio C Hamano
2023-09-14  5:31           ` Linus Arver
2023-09-09  6:16   ` [PATCH v2 5/6] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-09-11 18:54     ` Junio C Hamano
2023-09-14  2:41       ` Linus Arver
2023-09-14  3:16         ` Junio C Hamano
2023-09-22 18:23           ` Linus Arver
2023-09-22 19:48             ` Junio C Hamano
2023-09-26  5:30               ` Linus Arver
2023-09-09  6:16   ` [PATCH v2 6/6] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-11 19:01     ` Junio C Hamano
2023-09-14  1:21       ` Linus Arver
2023-09-14  3:18         ` Linus Arver
2023-09-22 19:50   ` [PATCH v3 0/9] Trailer readability cleanups Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 1/9] trailer: separate public from internal portion of trailer_iterator Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 2/9] trailer: split process_input_file into separate pieces Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 3/9] trailer: split process_command_line_args into separate functions Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 4/9] trailer: rename *_DEFAULT enums to *_UNSPECIFIED Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 5/9] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 6/9] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 7/9] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 8/9] trailer: only use trailer_block_* variables if trailers were found Linus Arver via GitGitGadget
2023-09-22 19:50     ` [PATCH v3 9/9] trailer: make stack variable names match field names Linus Arver via GitGitGadget
2023-09-22 22:47     ` [PATCH v3 0/9] Trailer readability cleanups Junio C Hamano
2023-09-22 23:13       ` Linus Arver
2023-09-23  0:48         ` Junio C Hamano
2023-09-26  5:40           ` Linus Arver
2023-09-26  6:22     ` [PATCH v4 0/4] " Linus Arver via GitGitGadget
2023-09-26  6:22       ` [PATCH v4 1/4] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-09-26  6:22       ` [PATCH v4 2/4] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-09-28 23:16         ` Jonathan Tan
2023-10-20  0:24           ` Linus Arver
2023-10-20  0:36             ` Junio C Hamano
2023-09-26  6:22       ` [PATCH v4 3/4] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget
2023-09-26  6:22       ` [PATCH v4 4/4] trailer: only use trailer_block_* variables if trailers were found Linus Arver via GitGitGadget
2023-10-20 19:01       ` [PATCH v5 0/3] Trailer readability cleanups Linus Arver via GitGitGadget
2023-10-20 19:01         ` [PATCH v5 1/3] commit: ignore_non_trailer computes number of bytes to ignore Linus Arver via GitGitGadget
2023-10-20 19:01         ` [PATCH v5 2/3] trailer: find the end of the log message Linus Arver via GitGitGadget
2023-10-20 21:29           ` Junio C Hamano
2023-12-29  6:42             ` Linus Arver [this message]
2023-12-29 21:03               ` Linus Arver
2023-10-20 19:01         ` [PATCH v5 3/3] trailer: use offsets for trailer_start/trailer_end Linus Arver via GitGitGadget

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=owlyv88hqzlu.fsf@fine.c.googlers.com \
    --to=linusa@google.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=glencbz@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=phillip.wood123@gmail.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).