Re: [PATCH 2/3] commit-graph: introduce `repo_find_commit_pos_in_graph()`

[Derrick Stolee <derrickstolee@github.com>]

On 7/12/2022 7:10 PM, Taylor Blau wrote:

> +int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,
> +				  uint32_t *pos)
> +{
> +	if (!prepare_commit_graph(r))
> +		return 0;
> +	return find_commit_pos_in_graph(c, r->objects->commit_graph, pos);
> +}
> +

This is absolutely correct for the given prototype.

>  void load_commit_graph_info(struct repository *r, struct commit *item)
>  {
>  	uint32_t pos;
> -	if (!prepare_commit_graph(r))
> -		return;
> -	if (find_commit_pos_in_graph(item, r->objects->commit_graph, &pos))
> +	if (repo_find_commit_pos_in_graph(r, item, &pos))
>  		fill_commit_graph_info(item, r->objects->commit_graph, pos);

Normally I would complain about referring directly to r->objects->commit_graph
without an explicit prepare_commit_graph() in the method. My initial thought
was that we would need to know that the new method prepares the graph, but
obviously the commit-graph needs to exist and be prepared if we are loading a
position from it.

All good here.

Thanks,
-Stolee