Re: [PATCH 05/10] evolve: add the change-table structure

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 27, 2022, 13:50 UTC
Message-ID: <220927.865yh8zzux.gmgdl@evledraar.gmail.com>
In-Reply-To: <3c61e0b3-5526-f42e-48a7-c4465d06ccb3@dunelm.org.uk>

On Tue, Sep 27 2022, Phillip Wood wrote:

Show 29 quoted lines

> On 23/09/2022 19:55, Stefan Xenos via GitGitGadget wrote:
>> From: Stefan Xenos <sxenos@google.com>
>> A change table stores a list of changes, and supports efficient
>> lookup
>> from a commit hash to the list of changes that reference that commit
>> directly.
>> It can be used to look up content commits or metacommits at the head
>> of a change, but does not support lookup of commits referenced as part
>> of the commit history.
>> Signed-off-by: Stefan Xenos <sxenos@google.com>
>> Signed-off-by: Chris Poucet <poucet@google.com>
>
>> diff --git a/change-table.h b/change-table.h
>> new file mode 100644
>> index 00000000000..166b5ed8073
>> --- /dev/null
>> +++ b/change-table.h
>> @@ -0,0 +1,132 @@
>> +#ifndef CHANGE_TABLE_H
>> +#define CHANGE_TABLE_H
>> +
>> +#include "oidmap.h"
>> +
>> +struct commit;
>> +struct ref_filter;
>> +
>> +/**
>
> We tend to just use '/*' rather than '/**'

No, we use both, and /** is correct here. It's an API-doc syntax, see e.g. strbuf.h.

It's explicitly meant for this sort of thing, i.e. comments on public structs in a header & the functions in a header (and struct members, etc.).

Show 12 quoted lines

>> + * This struct holds a list of change refs. The first element is
>   stored inline,
>> + * to optimize for small lists.
>> + */
>> +struct change_list {
>> +	/**
>> +	 * Ref name for the first change in the list, or null if none.
>> +	 *
>> +	 * This field is private. Use for_each_change_in to read.
>> +	 */
>> +	const char* first_refname;
>> +	/**

While we're on nits we tend to add an extra \n before the next API comment...