Re: [PATCH 1/9] t/unit-tests: adapt lib-reftable{c,h} helper functions to clar
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 29, 2025, 07:07 UTC
- Message-ID
- <aBB6vHl5Gmtva7b3@pks.im>
- In-Reply-To
- <CAGedMtfwA2vqOFxjLnusvFNcwKpTCLq38bZYBz-9cpzFmbhUdQ@mail.gmail.com>
On Tue, Apr 29, 2025 at 07:46:50AM +0100, Seyi Chamber wrote:
Show 21 quoted lines
> On Tue, 29 Apr 2025 at 00:27, Junio C Hamano <gitster@pobox.com> wrote:
> > With this step (and nothing else, as this is the first patch in the
> > series) applied to 'master', I see tons of these errors:
> >
> > CC t/unit-tests/t-reftable-merged.o
> > t/unit-tests/t-reftable-merged.c: In function 'merged_table_from_records':
> > t/unit-tests/t-reftable-merged.c:37:17: error: implicit declaration of function 't_reftable_write_to_buf'; did you mean 'cl_reftable_write_to_buf'? [-Wimplicit-function-declaration]
> > 37 | t_reftable_write_to_buf(&buf[i], refs[i], sizes[i], NULL, 0, &opts);
> > | ^~~~~~~~~~~~~~~~~~~~~~~
> > | cl_reftable_write_to_buf
> >
>
> Hi Junio,
>
> Yes, that is expected, as there are `reftable` test files that depend
> on the old function names. I navigated this by temporarily porting the
> functions into the `t/unit-tests/unit-test{c,h}` file to convert the
> files and moved them back as soon as I was done converting them. Would
> it be better to leave them in `t/unit-tests/unit-test{c,h}` and then
> create a final patch that migrates them into the
> `t/unit-tests/lib-reftable{c,h} file?We do want the Git history to be fully bisectable so that it is easy for contributors to find out where a specific bug has been introduced. And for the history to be bisectable, we must ensure that every single commit builds and passes our test suite. You can do that e.g. via `git rebase $BASE_COMMIT -x 'make -j$(nproc) test`, which builds and tests every single commit in your commit series.
So yes, this should be adapted so that the initial commit introduces the new helpers without removing the old helpers yet so that the unit tests that haven't yet been converted continue to build and test just fine.
Thanks!
Patrick