Re: [PATCH v2 00/19] Adds reftable library code from https://github.com/hanwen/reftable.

Han-Wen Nienhuys [off-list ref] writes:

On Thu, Sep 9, 2021 at 10:32 PM Junio C Hamano [off-list ref] wrote:

quoted

"Han-Wen Nienhuys via GitGitGadget" [off-list ref] writes:

quoted

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.

quoted

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.

On Mon, Sep 13, 2021 at 11:30 AM Junio C Hamano [off-list ref] wrote:

Han-Wen Nienhuys [off-list ref] writes:

quoted

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.

they will be flagged by the compiler in pedantic mode when in gnu89 mode though

  reftable/stack_test.c:49:1: warning: C++ style comments are not
allowed in ISO C
90
     49 | // Work linenumber into the tempdir, so we can see which
tests forget to
        | ^
  reftable/stack_test.c:49:1: note: (this will be reported only once
per input file)

and are probably an easy thing to fix

Carlo