On 4/6/2018 3:21 PM, Jeff King wrote:
On Fri, Apr 06, 2018 at 07:09:30PM +0000, Derrick Stolee wrote:
Derrick Stolee (4):
treewide: rename tree to maybe_tree
commit: create get_commit_tree() method
treewide: replace maybe_tree with accessor methods
commit-graph: lazy-load trees for commits
I gave this only a cursory read, but it addresses my concern from the
previous round.
If I were doing it myself, I probably would have folded patches 1 and 3
together. They are touching all the same spots, and it would be an error
for any case converted in patch 1 to not get converted in patch 3. I'm
assuming you caught them all due to Coccinelle, though IMHO it is
somewhat overkill here. By folding them together the compiler could tell
you which spots you missed.
And going forward, I doubt it is going to be a common error for people
to use maybe_tree directly. Between the name and the warning comment,
you'd have to really try to shoot yourself in the foot with it. The
primary concern was catching people using the existing "tree" name,
whose semantics changed.
All that said, I'm fine with having it done this way, too.
Thanks. As a double-check that I caught all of the 'maybe_tree'
accesses, I ran the following:
$ git grep maybe_tree | grep -v get_commit_tree
commit-graph.c: item->maybe_tree = NULL;
commit-graph.c: c->maybe_tree = lookup_tree(&oid);
commit-graph.c: return c->maybe_tree;
commit-graph.c: if (c->maybe_tree)
commit-graph.c: return c->maybe_tree;
commit.c: if (commit->maybe_tree || !commit->object.parsed)
commit.c: return commit->maybe_tree;
commit.c: item->maybe_tree = lookup_tree(&parent);
commit.h: struct tree *maybe_tree;
contrib/coccinelle/commit.cocci:- &c->maybe_tree->object.oid
contrib/coccinelle/commit.cocci:- c->maybe_tree->object.oid.hash
contrib/coccinelle/commit.cocci:- c->maybe_tree
contrib/coccinelle/commit.cocci:+ c->maybe_tree = s
contrib/coccinelle/commit.cocci:+ return c->maybe_tree;
merge-recursive.c: commit->maybe_tree = tree;
Thanks,
-Stolee
