Re: hn/reftable (Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))

From: Carlo Arenas <carenas@gmail.com>
To: Han-Wen Nienhuys <hanwen@google.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	git@vger.kernel.org, ramsay@ramsayjones.plus.com
Subject: Re: hn/reftable (Re: What's cooking in git.git (Sep 2021, #08; Mon, 27))
Date: Wed, 29 Sep 2021 22:06:02 -0700	[thread overview]
Message-ID: <CAPUEspj6q_N-ZPRzUv8jx_5_eP0gDO5TyTpi5-kPkfeoVhkjvg@mail.gmail.com> (raw)
In-Reply-To: <CAFQ2z_NxqLVfSWNtcXdyWQsTXrTrWGq7Qof=wdx9mmeqOAJS_Q@mail.gmail.com>

On Tue, Sep 28, 2021 at 5:19 AM Han-Wen Nienhuys <hanwen@google.com> wrote:
> On Tue, Sep 28, 2021 at 10:38 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > > * hn/reftable (2021-09-10) 20 commits
> > >  - fixup! reftable: implement stack, a mutable database of reftable files.
> > >  - Add "test-tool dump-reftable" command.
> > >  - reftable: add dump utility
> > >  - reftable: implement stack, a mutable database of reftable files.
> > >  - reftable: implement refname validation
> > >  - reftable: add merged table view
> > >  - reftable: add a heap-based priority queue for reftable records
> > >  - reftable: reftable file level tests
> > >  - reftable: read reftable files
> > >  - reftable: generic interface to tables
> > >  - reftable: write reftable files
> > >  - reftable: a generic binary tree implementation
> > >  - reftable: reading/writing blocks
> > >  - Provide zlib's uncompress2 from compat/zlib-compat.c
> > >  - reftable: (de)serialization for the polymorphic record type.
> > >  - reftable: add blocksource, an abstraction for random access reads
> > >  - reftable: utility functions
> > >  - reftable: add error related functionality
> > >  - reftable: RFC: add LICENSE
> > >  - hash.h: provide constants for the hash IDs
> > >
> > >  The "reftable" backend for the refs API, without integrating into
> > >  the refs subsystem.
> > >
> > >  Will merge to 'next'?
> >
> > I think we've reached approximately "good enough" with this for the next
> > steps, and can hopefully fix any remaining nits (such as my [1])
> > post-merge.
>
> There is still an RFC in 02/19. Maybe we can get agreement that this
> is OK and drop the RFC ?
>
> > Maybe hold off until Han-Wen gets a chance to ack it, and whether he's
> > ok with the proposed fixup(s)?
>
> regarding uncompress2: I thought it was functionality best left to
> zlib to implement, but I imagine git.git offers something similar.
> Sounds good to use that.
>
> > Wasn't there an outstanding "some of this in reftable/* should be static
> > functions" from someone, Carlo? (CC'd). In any case that sort of thing
> > could also be a post-cleanup, I couldn't find a reference to that
> > discussion in anything except my vague memory of it as I wrote this.
>
> I think I have addressed all outstanding issues in my github PR, and
> I'll send it out once I see CI pass.

It was actually raised by Ramsay[1], and might have been a red herring,
but there was for sure no follow up.

I have a few minor fixups that will post for discussion, and might be even
ok to merge to next without squashing if included on top, but I am not
sure if all those issues could be addressed post-cleanup, since IMHO
once this gets into master, then it will be considered a stable API
and it can't be changed.

For example. while looking at Ramsay message noticed the API for "reader"
in libreftable might be better with s/init_reader/reader_init/g

Carlo

[1] https://lore.kernel.org/git/bc387e32-321e-4726-2a02-2e6cf6ed5250@ramsayjones.plus.com/

CC +Ramsay