From: Junio C Hamano <gitster@pobox.com>
To: Han-Wen Nienhuys <hanwen@google.com>
Cc: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>,
git@vger.kernel.org,
"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
"Han-Wen Nienhuys" <hanwenn@gmail.com>
Subject: Re: [PATCH v2 00/19] Adds reftable library code from https://github.com/hanwen/reftable.
Date: Mon, 13 Sep 2021 11:30:49 -0700 [thread overview]
Message-ID: <xmqqee9sfhuu.fsf@gitster.g> (raw)
In-Reply-To: <CAFQ2z_Pa+KmCYV224XwMXO1iFCNA=PXj5iKaQU3LYGYTK_+qsw@mail.gmail.com> (Han-Wen Nienhuys's message of "Mon, 13 Sep 2021 12:14:34 +0200")
Han-Wen Nienhuys <hanwen@google.com> writes:
> On Thu, Sep 9, 2021 at 10:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > The reftable format is described in Documentation/technical/reftable.txt.
>> >
>> > This is a fully reentrant implementation of reading and writing the reftable
>> > file format, and should be suitable for embedding in libgit2 too. It does
>> > not hook the code up to git to function as a ref storage backend yet.
>>
>> Not a question for Han-Wen, but I am wondering how much style and
>> other consistency guidelines we have in our C code to the files in
>> this directory.
>
> I am Han-Wen, but I'm not sure what you are saying here.
Sorry, the sentence is unreadable because I missed a verb above
(insert "should apply" between "code" and "to").
I was asking for opinions on how we should treat this piece of code.
We loosen "style guidelines" on borrowed code that is maintained
elsewhere to ease re-importing, but code we maintain in-tree are
subject to our style guide. I am not sure how this part that is
meant to be reusable in other projects like libgit2 should be
treated.
>> I am guessing that rules like "no decl after statement" and "no decl
>> in the set-up part of the for loop control" (i.e. "for (int i = 0;
>> ..." is a no-no) should apply equally to this code, but it might be
>> OK to deviate from rules that are only meant to help human readers [*]
>> without affecting compilation.
>>
>> Opinions?
>
> The code has a different style because I wrote it separately from Git.
> I'm not wedded to its current style, and most styling can easily be
> changed. If you have specific things that should be addressed, let me
> know.
The question was for other reviewers to help us come up with what
the "specific things" ought to be. I saw style differences around
comments and code formatting (everything I listed in the footnote,
plus, // comment which I forgot to mention) which may or may not
turn out to be part of that "specific things", because they do not
break compilation.
next prev parent reply other threads:[~2021-09-13 18:31 UTC|newest]
Thread overview: 111+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-30 14:57 [PATCH 00/19] Adds reftable library code from https://github.com/hanwen/reftable Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 01/19] hash.h: provide constants for the hash IDs Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 02/19] reftable: RFC: add LICENSE Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 03/19] reftable: add error related functionality Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 04/19] reftable: utility functions Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 05/19] reftable: add blocksource, an abstraction for random access reads Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 06/19] reftable: (de)serialization for the polymorphic record type Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 07/19] Provide zlib's uncompress2 from compat/zlib-compat.c Han-Wen Nienhuys via GitGitGadget
2021-09-02 6:12 ` [PATCH] fixup! " Carlo Marcelo Arenas Belón
2021-08-30 14:57 ` [PATCH 08/19] reftable: reading/writing blocks Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 09/19] reftable: a generic binary tree implementation Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 10/19] reftable: write reftable files Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 11/19] reftable: generic interface to tables Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 12/19] reftable: read reftable files Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 13/19] reftable: reftable file level tests Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 14/19] reftable: add a heap-based priority queue for reftable records Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 15/19] reftable: add merged table view Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 16/19] reftable: implement refname validation Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 17/19] reftable: implement stack, a mutable database of reftable files Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 18/19] reftable: add dump utility Han-Wen Nienhuys via GitGitGadget
2021-08-30 14:57 ` [PATCH 19/19] Add "test-tool dump-reftable" command Han-Wen Nienhuys via GitGitGadget
2021-08-30 15:22 ` [PATCH 00/19] Adds reftable library code from https://github.com/hanwen/reftable Han-Wen Nienhuys
2021-09-08 7:45 ` [PATCH 0/4] fixup for hn/reftable Carlo Marcelo Arenas Belón
2021-09-08 7:45 ` [PATCH 1/4] fixup! reftable: reading/writing blocks Carlo Marcelo Arenas Belón
2021-09-08 7:45 ` [PATCH 2/4] fixup! reftable: utility functions Carlo Marcelo Arenas Belón
2021-09-08 7:45 ` [PATCH 3/4] fixup! Provide zlib's uncompress2 from compat/zlib-compat.c Carlo Marcelo Arenas Belón
2021-09-08 7:45 ` [PATCH 4/4] fixup! reftable: utility functions Carlo Marcelo Arenas Belón
2021-09-08 18:50 ` [PATCH 0/4] fixup for hn/reftable Junio C Hamano
2021-09-09 18:47 ` [PATCH v2 00/19] Adds reftable library code from https://github.com/hanwen/reftable Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 01/19] hash.h: provide constants for the hash IDs Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 02/19] reftable: RFC: add LICENSE Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 03/19] reftable: add error related functionality Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 04/19] reftable: utility functions Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 05/19] reftable: add blocksource, an abstraction for random access reads Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 06/19] reftable: (de)serialization for the polymorphic record type Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 07/19] Provide zlib's uncompress2 from compat/zlib-compat.c Han-Wen Nienhuys via GitGitGadget
2021-09-15 7:34 ` Carlo Arenas
2021-09-09 18:47 ` [PATCH v2 08/19] reftable: reading/writing blocks Han-Wen Nienhuys via GitGitGadget
2021-09-24 11:52 ` Ævar Arnfjörð Bjarmason
2021-09-09 18:47 ` [PATCH v2 09/19] reftable: a generic binary tree implementation Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 10/19] reftable: write reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 11/19] reftable: generic interface to tables Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 12/19] reftable: read reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 13/19] reftable: reftable file level tests Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 14/19] reftable: add a heap-based priority queue for reftable records Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 15/19] reftable: add merged table view Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 16/19] reftable: implement refname validation Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 17/19] reftable: implement stack, a mutable database of reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 18/19] reftable: add dump utility Han-Wen Nienhuys via GitGitGadget
2021-09-09 18:47 ` [PATCH v2 19/19] Add "test-tool dump-reftable" command Han-Wen Nienhuys via GitGitGadget
2021-09-09 20:02 ` [PATCH v2 00/19] Adds reftable library code from https://github.com/hanwen/reftable Junio C Hamano
2021-09-09 20:32 ` Junio C Hamano
2021-09-13 10:14 ` Han-Wen Nienhuys
2021-09-13 18:30 ` Junio C Hamano [this message]
2021-09-13 19:29 ` Carlo Arenas
2021-09-13 20:34 ` Junio C Hamano
2021-09-28 15:09 ` [PATCH v3 " Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:09 ` [PATCH v3 01/19] hash.h: provide constants for the hash IDs Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:09 ` [PATCH v3 02/19] reftable: RFC: add LICENSE Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 03/19] reftable: add error related functionality Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 04/19] reftable: utility functions Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 05/19] reftable: add blocksource, an abstraction for random access reads Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 06/19] reftable: (de)serialization for the polymorphic record type Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 07/19] Provide zlib's uncompress2 from compat/zlib-compat.c Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 08/19] reftable: reading/writing blocks Han-Wen Nienhuys via GitGitGadget
2021-09-30 12:23 ` [PATCH] squash! " Carlo Marcelo Arenas Belón
2021-10-07 16:34 ` Han-Wen Nienhuys
2021-09-28 15:10 ` [PATCH v3 09/19] reftable: a generic binary tree implementation Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 10/19] reftable: write reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 11/19] reftable: generic interface to tables Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 12/19] reftable: read reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 13/19] reftable: reftable file level tests Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 14/19] reftable: add a heap-based priority queue for reftable records Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 15/19] reftable: add merged table view Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 16/19] reftable: implement refname validation Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 17/19] reftable: implement stack, a mutable database of reftable files Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 18/19] reftable: add dump utility Han-Wen Nienhuys via GitGitGadget
2021-09-28 15:10 ` [PATCH v3 19/19] Add "test-tool dump-reftable" command Han-Wen Nienhuys via GitGitGadget
2021-09-28 18:17 ` [PATCH v3 00/19] Adds reftable library code from https://github.com/hanwen/reftable Junio C Hamano
2021-10-02 9:20 ` Ævar Arnfjörð Bjarmason
2021-09-30 5:40 ` hn/reftable "fixes" Carlo Marcelo Arenas Belón
2021-09-30 5:40 ` [PATCH 1/4] fixup! reftable: add a heap-based priority queue for reftable records Carlo Marcelo Arenas Belón
2021-09-30 5:40 ` [PATCH 2/4] fixup! reftable: implement stack, a mutable database of reftable files Carlo Marcelo Arenas Belón
2021-10-01 15:37 ` C++(C99)-style comments in git.git Ævar Arnfjörð Bjarmason
2021-09-30 5:40 ` [PATCH 3/4] config.mak.uname: last release and snapshots of Minix still use zlib 1.2.3 Carlo Marcelo Arenas Belón
2021-09-30 5:40 ` [PATCH 4/4] reftable: avoid non portable compile time pointer to function Carlo Marcelo Arenas Belón
2021-09-30 20:35 ` hn/reftable "fixes" Junio C Hamano
2021-10-07 20:24 ` [PATCH v4 00/19] Adds reftable library code from https://github.com/hanwen/reftable Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:24 ` [PATCH v4 01/19] hash.h: provide constants for the hash IDs Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:24 ` [PATCH v4 02/19] reftable: add LICENSE Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:24 ` [PATCH v4 03/19] reftable: add error related functionality Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 04/19] reftable: utility functions Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 05/19] reftable: add blocksource, an abstraction for random access reads Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 06/19] reftable: (de)serialization for the polymorphic record type Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 07/19] Provide zlib's uncompress2 from compat/zlib-compat.c Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 08/19] reftable: reading/writing blocks Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 09/19] reftable: a generic binary tree implementation Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 10/19] reftable: write reftable files Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 11/19] reftable: generic interface to tables Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 12/19] reftable: read reftable files Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 13/19] reftable: reftable file level tests Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 14/19] reftable: add a heap-based priority queue for reftable records Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 15/19] reftable: add merged table view Han-Wen Nienhuys via GitGitGadget
2022-01-13 11:38 ` [PATCH] reftable tests: use C syntax compatible with old xlc Ævar Arnfjörð Bjarmason
2022-01-13 14:23 ` Han-Wen Nienhuys
2022-01-13 16:22 ` Ævar Arnfjörð Bjarmason
2022-01-13 19:09 ` Junio C Hamano
2021-10-07 20:25 ` [PATCH v4 16/19] reftable: implement refname validation Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 17/19] reftable: implement stack, a mutable database of reftable files Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 18/19] reftable: add dump utility Han-Wen Nienhuys via GitGitGadget
2021-10-07 20:25 ` [PATCH v4 19/19] Add "test-tool dump-reftable" command Han-Wen Nienhuys 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
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=xmqqee9sfhuu.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=hanwen@google.com \
--cc=hanwenn@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.
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).