On Tue, Apr 29, 2025 at 07:46:50AM +0100, Seyi Chamber wrote:

On Tue, 29 Apr 2025 at 00:27, Junio C Hamano [off-list ref] wrote:

quoted

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