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

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