Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag <snip>

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 21, 2019, 12:11 UTC
Message-ID: <20190921121104.GA6787@szeder.dev>
In-Reply-To: <6badd9ceaf4851b2984e78a5cfd0cb8ec0c810f5.1568998427.git.gitgitgadget@gmail.com>

On Fri, Sep 20, 2019 at 09:53:48AM -0700, William Baker via GitGitGadget wrote:

Show 9 quoted lines

> diff --git a/midx.h b/midx.h
> index f0ae656b5d..e6fa356b5c 100644
> --- a/midx.h
> +++ b/midx.h
> @@ -37,6 +37,8 @@ struct multi_pack_index {
>  	char object_dir[FLEX_ARRAY];
>  };
>  
> +#define MIDX_PROGRESS     (1 << 0)

Please consider using an enum.

A preprocessor constant is just that: a number. It is shown as a number while debugging, and there is no clear indication what related values belong in the same group (apart from the prefix of the macro name), or what possible values an 'unsigned flags' variable might have (though in this particular case there is only a single possible value...).

An enum, however, is much friendlier to humans when debugging, because even 'gdb' shows the value using the symbolic names, e.g.:

  write_commit_graph_reachable (obj_dir=0x9ab870 ".git/objects", 
      flags=(COMMIT_GRAPH_WRITE_APPEND | COMMIT_GRAPH_WRITE_PROGRESS), 
      split_opts=0x9670e0 <split_opts>) at commit-graph.c:1141

and it's quite clear what values a variable of type 'enum commit_graph_write_flags' can have.