Taylor Blau <me@xxxxxxxxxxxx> writes: > -static int commit_graft_pos(struct repository *r, const unsigned char *sha1) > +int commit_graft_pos(struct repository *r, const unsigned char *sha1) > { > return sha1_pos(sha1, r->parsed_objects->grafts, > r->parsed_objects->grafts_nr, > diff --git a/commit.h b/commit.h > index ab91d21131..eb42e8b6d2 100644 > --- a/commit.h > +++ b/commit.h > @@ -236,6 +236,7 @@ struct commit_graft { > typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); > > struct commit_graft *read_graft_line(struct strbuf *line); > +int commit_graft_pos(struct repository *r, const unsigned char *sha1); In an earlier exchange, I saw this: >> - could include a comment saying that it's an index into >> r->parsed_objects->grafts > > This and the below are both good ideas to me. I prefer this one, since > we'd have to duplicate yet another static function > ('commit_graft_sha1_access()' directly above) that is called by this > one. > >> - I'm usually loathe to suggest unnecessary duplication of code, but >> it might make sense to duplicate the function into shallow.c. Or >> even to inline it there (in the single call site, that ends up >> being pretty readable). > > I am not at all offended by duplication of code where it makes sense to > do so, but having to duplicate two functions seems like we'd be better > off simply documenting the function in commit.h. and I think I agree with that direction. Forgot to add those comments?