git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Karthik Nayak <karthik.188@gmail.com>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH v2 02/16] odb: move list of packfiles into `struct packfile_store`
Date: Tue, 2 Sep 2025 13:21:04 -0400	[thread overview]
Message-ID: <aLcngI6vm9DmqUeI@nand.local> (raw)
In-Reply-To: <aLav4UAcfQjvNzMF@pks.im>

On Tue, Sep 02, 2025 at 10:50:41AM +0200, Patrick Steinhardt wrote:
> > > +void packfile_store_close(struct packfile_store *store)
> > > +{
> > > +	struct packed_git *p;
> > > +
> > > +	for (p = store->packs; p; p = p->next)
> > > +		if (p->do_not_close)
> > > +			BUG("want to close pack marked 'do-not-close'");
> > > +		else
> > > +			close_pack(p);
> > > +}
> >
> > And likewise this looks good to me. I do find the braceless for-loop a
> > little hard to read, but it's (a) correct, and (b) consistent with the
> > original implementation, so I don't feel strongly about changing it.
>
> Agreed, it is a bit awkward. I feel like our coding style should be
> amended to say that we only do braceless bodies in case the body is a
> single statement.

I think that our CodingGuidelines cover this as of 1797dc5176
(CodingGuidelines: clarify multi-line brace style, 2017-01-17), which
frowns upon statements that extend multiple lines.

So I think in this case, the CodingGuidelines would suggest that we
write this as:

    for (p = store->packs; p; p = p->next) {
        if (p->do_not_close)
            BUG("want to close pack marked 'do-not-close'");
        else
            close_pack(p);
    }

, which from our discussion here seems like something that we both find
more readable than the original.

Thanks,
Taylor

  reply	other threads:[~2025-09-02 17:21 UTC|newest]

Thread overview: 181+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-19  8:19 [PATCH 00/16] packfile: carve out a new packfile store Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 01/16] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-08-19  9:47   ` Karthik Nayak
2025-08-20  4:58     ` Patrick Steinhardt
2025-08-19 17:32   ` Junio C Hamano
2025-08-20  4:58     ` Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 02/16] odb: move list of packfiles into " Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 03/16] odb: move initialization bit " Patrick Steinhardt
2025-08-19  9:57   ` Karthik Nayak
2025-08-19 16:24     ` Junio C Hamano
2025-08-20  8:04       ` Karthik Nayak
2025-08-22 23:50         ` Junio C Hamano
2025-08-26 12:19           ` [PATCH] Documentation: note styling for bit fields Karthik Nayak
2025-08-20  4:58     ` [PATCH 03/16] odb: move initialization bit into `struct packfile_store` Patrick Steinhardt
2025-08-20  6:24       ` Junio C Hamano
2025-08-19  8:19 ` [PATCH 04/16] odb: move packfile map " Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 05/16] odb: move MRU list of packfiles " Patrick Steinhardt
2025-08-20 12:44   ` Karthik Nayak
2025-08-20 19:20     ` Jeff King
2025-08-21  6:40       ` Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 06/16] odb: move kept cache " Patrick Steinhardt
2025-08-19 18:56   ` Junio C Hamano
2025-08-20  4:58     ` Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 07/16] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-08-19 19:18   ` Junio C Hamano
2025-08-19  8:19 ` [PATCH 08/16] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 09/16] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-08-20 13:17   ` Karthik Nayak
2025-08-19  8:19 ` [PATCH 10/16] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 11/16] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-08-20 13:35   ` Karthik Nayak
2025-08-19  8:19 ` [PATCH 12/16] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-08-20 13:41   ` Karthik Nayak
2025-08-21  6:40     ` Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 13/16] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 14/16] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-08-20 13:50   ` Karthik Nayak
2025-08-21  6:40     ` Patrick Steinhardt
2025-08-20 13:51   ` Karthik Nayak
2025-08-19  8:19 ` [PATCH 15/16] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-08-20 13:53   ` Karthik Nayak
2025-08-21  6:40     ` Patrick Steinhardt
2025-08-19  8:19 ` [PATCH 16/16] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-08-19 17:13 ` [PATCH 00/16] packfile: carve out a new " Junio C Hamano
2025-08-20 13:55 ` Karthik Nayak
2025-08-21  7:38 ` [PATCH v2 " Patrick Steinhardt
2025-08-21  7:38   ` [PATCH v2 01/16] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 02/16] odb: move list of packfiles into " Patrick Steinhardt
2025-08-25 23:42     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-09-02 17:21         ` Taylor Blau [this message]
2025-09-02 17:42           ` Junio C Hamano
2025-09-03  5:58             ` Patrick Steinhardt
2025-09-11 23:16         ` Taylor Blau
2025-09-15  7:44           ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 03/16] odb: move initialization bit " Patrick Steinhardt
2025-08-26  1:40     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 04/16] odb: move packfile map " Patrick Steinhardt
2025-08-26  1:41     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 05/16] odb: move MRU list of packfiles " Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 06/16] odb: move kept cache " Patrick Steinhardt
2025-08-26  1:46     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 07/16] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-08-26  1:47     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 08/16] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-26  1:58     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 09/16] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-08-26  2:10     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 10/16] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-08-26  2:11     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 11/16] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-08-27  1:04     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 12/16] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-08-27  1:12     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 13/16] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-08-27  1:20     ` Taylor Blau
2025-08-21  7:39   ` [PATCH v2 14/16] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-08-27  1:38     ` Taylor Blau
2025-09-02  8:50       ` Patrick Steinhardt
2025-09-11 23:25         ` Taylor Blau
2025-09-15  7:30           ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 15/16] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-08-27  1:45     ` Taylor Blau
2025-09-02  8:51       ` Patrick Steinhardt
2025-09-11 23:33         ` Taylor Blau
2025-09-15  7:44           ` Patrick Steinhardt
2025-08-21  7:39   ` [PATCH v2 16/16] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-02 10:48 ` [PATCH v3 00/15] packfile: carve out a new " Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-09  7:49     ` Karthik Nayak
2025-09-02 10:48   ` [PATCH v3 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-09  8:00     ` Karthik Nayak
2025-09-09 11:09       ` Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-09  8:22     ` Karthik Nayak
2025-09-09 11:01       ` Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 13/15] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 14/15] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-09-02 10:48   ` [PATCH v3 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-02 16:40   ` [PATCH v3 00/15] packfile: carve out a new " Junio C Hamano
2025-09-11 23:34     ` Taylor Blau
2025-09-09  9:33   ` Karthik Nayak
2025-09-09 11:02 ` [PATCH v4 " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 13/15] packfile: remove `get_packed_git()` Patrick Steinhardt
2025-09-11 23:37     ` Taylor Blau
2025-09-09 11:03   ` [PATCH v4 14/15] packfile: refactor `get_all_packs()` to work on packfile store Patrick Steinhardt
2025-09-09 11:03   ` [PATCH v4 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-10  7:35   ` [PATCH v4 00/15] packfile: carve out a new " Karthik Nayak
2025-09-11 23:40   ` Taylor Blau
2025-09-11 23:42     ` Taylor Blau
2025-09-15  7:25       ` Patrick Steinhardt
2025-09-15  8:54 ` [PATCH v5 " Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-17 21:26     ` Justin Tobler
2025-09-23  9:34       ` Patrick Steinhardt
2025-09-24 21:56         ` Justin Tobler
2025-09-15  8:54   ` [PATCH v5 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-17 22:15     ` Justin Tobler
2025-09-23  9:35       ` Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-17 21:59     ` Justin Tobler
2025-09-15  8:54   ` [PATCH v5 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-17 22:32     ` Justin Tobler
2025-09-23  9:34       ` Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 13/15] packfile: refactor `get_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 14/15] packfile: refactor `get_all_packs()` " Patrick Steinhardt
2025-09-15  8:54   ` [PATCH v5 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-23 10:16 ` [PATCH v6 00/15] packfile: carve out a new " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 01/15] packfile: introduce a new `struct packfile_store` Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 02/15] odb: move list of packfiles into " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 03/15] odb: move initialization bit " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 04/15] odb: move packfile map " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 05/15] odb: move MRU list of packfiles " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 06/15] odb: move kept cache " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 07/15] packfile: reorder functions to avoid function declaration Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 08/15] packfile: refactor `prepare_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 09/15] packfile: split up responsibilities of `reprepare_packed_git()` Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 10/15] packfile: refactor `install_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 11/15] packfile: introduce function to load and add packfiles Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 12/15] packfile: move `get_multi_pack_index()` into "midx.c" Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 13/15] packfile: refactor `get_packed_git()` to work on packfile store Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 14/15] packfile: refactor `get_all_packs()` " Patrick Steinhardt
2025-09-23 10:17   ` [PATCH v6 15/15] packfile: refactor `get_packed_git_mru()` " Patrick Steinhardt
2025-09-24 21:58   ` [PATCH v6 00/15] packfile: carve out a new " Justin Tobler
2025-09-25 16:08   ` Junio C Hamano
2025-09-26  5:26     ` Patrick Steinhardt
2025-09-28 22:05       ` Taylor Blau
2025-09-29 21:39         ` Patrick Steinhardt

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=aLcngI6vm9DmqUeI@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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 NNTP newsgroup(s).