"Derrick Stolee via GitGitGadget" <gitgitgad...@gmail.com> writes:

> +     int version = 0;
> ...
> +     if (flags & COMMIT_GRAPH_VERSION_1)
> +             version = 1;
> +     if (!version)
> +             version = 1;
> +     if (version != 1) {
> +             error(_("unsupported commit-graph version %d"),
> +                   version);
> +             return 1;
> +     }

The above sequence had a certain "Huh?" factor before 5/5 introduced
the support for a later version that is in use by default.

Is it sensible to define VERSION_$N as if they are independent bits
in a single flags variable?  What does it mean for the flags variable
to have both GRAPH_VERSION_1 and GRAPH_VERSION_2 bits set?

What I am getting at is if this is better done as a n-bit bitfield
that represents a small unsigned integer (e.g. "unsigned char" that
lets you play with up to 255 versions, or "unsigned version : 3"
that limits you to up to 7 versions).

You use an 8-bit byte in the file format anyway, so it might not be
so bad to have a separate version parameter that is not mixed with
the flag bits, perhaps?

Reply via email to