Subject: Re: [PATCH v3] commit-graph: remove a duplicate assignment

From: Junio C Hamano <[email-masked]>
Date: Mon, 30 Sep 2019 10:22:44 +0900

Alex Henrie <[email-masked]> writes:

> The variable g was being set to the same value both at the beginning of
> the function and before the loop. The assignment before the loop was
> kept because it helps clarify what the loop does, and the redundant
> assignment at the beginning of the function was removed.

Writing these mostly in the past tense is misleading to those who
are used to read "git log" from this project.  Give orders to the
codebase to "become like so" instead.  Perhaps like

	Leave the variable 'g' uninitialized before it is set just
	before its first use in front of a loop, which is a lot more
	appropriate place to indicate what it is used for.

>  static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
>  {
> -	struct commit_graph *g = ctx->r->objects->commit_graph;
> +	struct commit_graph *g;
>  	uint32_t num_commits = ctx->commits.nr;

Stepping back a bit, doesn't the same justification you gave to this
change apply to 'num_commits'?  If you make it uninitialized before
its first use and assign ctx->commits.nr to it near where 'g' is
given its first value, wouldn't it make it even clearer that these
two variables are almost always used together and how they are used
in the loop?