git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Christian Couder <chriscool@tuxfamily.org>,
	 Emily Shaffer <nasamuffin@google.com>,
	 Josh Steadmon <steadmon@google.com>,
	"Randall S. Becker" <rsbecker@nexbridge.com>,
	 Christian Couder <christian.couder@gmail.com>,
	 Kristoffer Haugsbakk <code@khaugsbakk.name>,
	 Linus Arver <linusa@google.com>
Subject: Re: [PATCH v2 6/8] trailer: make parse_trailers() return trailer_info pointer
Date: Tue, 23 Apr 2024 16:17:08 -0700	[thread overview]
Message-ID: <xmqq8r136557.fsf@gitster.g> (raw)
In-Reply-To: <093f68f365801bc8801f29dc3e6eaa9b07fcd57d.1713504153.git.gitgitgadget@gmail.com> (Linus Arver via GitGitGadget's message of "Fri, 19 Apr 2024 05:22:31 +0000")

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> ... From the trailer API user's perspective, the call
> to trailer_info_new() can be replaced with parse_trailers(); do so in
> interpret-trailers.

And from the trailer API users' perspective, it would now help to
have a bit of comment on parse_trailers() function.  The users need
to know at least:

 - what the function returns (i.e. a pointer to an opaque
   trailer_info structure),

 - what operations can be done to the opaque structure, and 

 - what informations can be extracted out of the opaque structure.

When appropriately typed and named members in the structure are
visible, the latter two are obvious in well written programs, but
now you are going to hide the data structure, "Show me your data
structures, and I won't usually need your code; it'll be obvious."
would no longer work.  You'd need to compensate for making the
structure opaque to rob the clarity from the readers with good
comments to the function and the API to help them.

The changes in this patch look more-or-less obvious.  Looking good.


  reply	other threads:[~2024-04-23 23:17 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-16  6:27 [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 1/6] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 2/6] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 3/6] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 4/6] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 5/6] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-03-16  6:27 ` [PATCH 6/6] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-03-16 17:06 ` [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-03-26 22:00 ` Junio C Hamano
2024-04-19  5:36   ` Linus Arver
2024-04-19  5:22 ` [PATCH v2 0/8] " Linus Arver via GitGitGadget
2024-04-19  5:22   ` [PATCH v2 1/8] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-19  5:22   ` [PATCH v2 2/8] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-19  5:33     ` Linus Arver
2024-04-19 18:46     ` Linus Arver
2024-04-19 21:52     ` Junio C Hamano
2024-04-20  0:14       ` Linus Arver
2024-04-19  5:22   ` [PATCH v2 3/8] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-19  5:22   ` [PATCH v2 4/8] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-23 21:19     ` Junio C Hamano
2024-04-19  5:22   ` [PATCH v2 5/8] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-19  5:22   ` [PATCH v2 6/8] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-23 23:17     ` Junio C Hamano [this message]
2024-04-19  5:22   ` [PATCH v2 7/8] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-23 23:27     ` Junio C Hamano
2024-04-25  3:17       ` Linus Arver
2024-04-19  5:22   ` [PATCH v2 8/8] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-23 23:27     ` Junio C Hamano
2024-04-24  0:27   ` [PATCH v2 0/8] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-04-26  0:26   ` [PATCH v3 00/10] " Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-26 14:51       ` Christian Couder
2024-04-26 16:20         ` Junio C Hamano
2024-04-26 16:25         ` Linus Arver
2024-04-26  0:26     ` [PATCH v3 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-27 12:50       ` Christian Couder
2024-04-30  4:42         ` Linus Arver
2024-04-30  4:55           ` Linus Arver
2024-04-26  0:26     ` [PATCH v3 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-04-26  0:26     ` [PATCH v3 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-04-27 12:51     ` [PATCH v3 00/10] Make trailer_info struct private (plus sequencer cleanup) Christian Couder
2024-05-02  4:54     ` [PATCH v4 " Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-05-02 16:54         ` Junio C Hamano
2024-05-02  4:54       ` [PATCH v4 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-05-04 15:33         ` Phillip Wood
2024-05-05  1:37           ` Linus Arver
2024-05-05 14:09             ` Phillip Wood
2024-05-09  7:11               ` Linus Arver
2024-05-13 15:11                 ` Phillip Wood
2024-05-13 15:13                   ` Phillip Wood
2024-05-02  4:54       ` [PATCH v4 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-05-02  4:54       ` [PATCH v4 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-05-02 17:15       ` [PATCH v4 00/10] Make trailer_info struct private (plus sequencer cleanup) 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq8r136557.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=linusa@google.com \
    --cc=nasamuffin@google.com \
    --cc=rsbecker@nexbridge.com \
    --cc=steadmon@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).