Re: [PATCH v4 03/11] commit-graph: collapse parameters into flags

[Junio C Hamano <gitster@pobox.com>]

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

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The write_commit_graph() and write_commit_graph_reachable() methods
> currently take two boolean parameters: 'append' and 'report_progress'.
> We will soon expand the possible options to send to these methods, so
> instead of complicating the parameter list, first simplify it.

I think this change to introduce "flags" and pack these two into a
single parameter, even if there is no plan to add code that starts
using third and subsequent bits immediately.

We are no longer adding anything beyond PROGRESS and APPEND in this
series, no?

>
> Collapse these parameters into a 'flags' parameter, and adjust the
> callers to provide flags as necessary.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/commit-graph.c | 8 +++++---
>  builtin/commit.c       | 2 +-
>  builtin/gc.c           | 4 ++--
>  commit-graph.c         | 9 +++++----
>  commit-graph.h         | 8 +++++---
>  5 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> index 2e86251f02..828b1a713f 100644
> --- a/builtin/commit-graph.c
> +++ b/builtin/commit-graph.c
> @@ -142,6 +142,7 @@ static int graph_write(int argc, const char **argv)
>  	struct string_list *commit_hex = NULL;
>  	struct string_list lines;
>  	int result;
> +	int flags = COMMIT_GRAPH_PROGRESS;

Make it a habit to use "unsigned" not a signed type, when you pack a
collection of bits into a flag word, unless you are treating the MSB
specially, e.g. checking to see if it is negative is cheaper than
masking with MSB to see if it is set.

> ...
>  	result = write_commit_graph(opts.obj_dir,
>  				    pack_indexes,
>  				    commit_hex,
> -				    opts.append,
> -				    1);
> +				    flags);
> ...
> -int write_commit_graph_reachable(const char *obj_dir, int append,
> -				 int report_progress)
> +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags)
> ...
>  int write_commit_graph(const char *obj_dir,
>  		       struct string_list *pack_indexes,
>  		       struct string_list *commit_hex,
> -		       int append, int report_progress)
> +		       unsigned int flags)

OK, so the receivers of the flags word know the collection is
unsigned; it's just the user of the API in graph_write() that gets
the signedness wrong.  OK, easy enough to correct, I guess.