"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> +enum sparse_index_mode {
> + /*
> + * COMPLETELY_FULL: there are no sparse directories
> + * in the index at all.
> + */
> + COMPLETELY_FULL = 0,
Two things that make me wonder are:
* Do concrete values assigned to these symbols matter? If we do
if (some_function_that_returns_this_type())
/* ok, we know it is full */
do_full_thing();
then the answer is yes. If we write these values to on-disk
file, and other versions and reimplementations of Git need to
know the concrete values, then the answer is yes. Otherwise, we
may not want to say "= 0" and the like here.
* Is it worth repeating the label in the comment? IOW, I am
wondering if
/* there are no sparse directories in the index */
COMPLETELY_FULL,
is equally readable and slightly more maintainable.
Also these names being in cache.h and visible everywhere is somewhat
worrying. Other CPP macros and enum constants in the file have
names that are prepared to be truly global, but COMPLETELY_FULL and
COLLAPSED do not hint that they are about sparse-index state as
strongly as PARTIALLY_SPARSE does.
> - /*
> - * sparse_index == 1 when sparse-directory
> - * entries exist. Requires sparse-checkout
> - * in cone mode.
> - */
> - sparse_index : 1;
> + fsmonitor_has_run_once : 1;
> + enum sparse_index_mode sparse_index;
Good that we can lose the comment on a single bit. Are some of the
enum sparse_index_mode values only possible in cone mode, just like
having true in this bit was only possible in cone mode? Perhaps a
comment before "enum sparse_index_mode {" can say "in non-cone mode,
COMPLETELY_FULL is the only possible value" or somesuch?
Thanks.
