Brian Lyles <brianmlyles@xxxxxxxxx> writes:
> + if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
> + /*
> + * Check to see if this is an unborn branch
> + */
> + head_name = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, &head_oid, NULL);
> + if (!head_name || !starts_with(head_name, "refs/heads/") || !is_null_oid(&head_oid))
> + return error(_("could not resolve HEAD commit"));
> + head_tree_oid = the_hash_algo->empty_tree;
> + } else {
> + head_commit = lookup_commit(r, &head_oid);
>
> - head_commit = lookup_commit(r, &head_oid);
> + /*
> + * If head_commit is NULL, check_commit, called from
> + * lookup_commit, would have indicated that head_commit is not
> + * a commit object already. repo_parse_commit() will return failure
> + * without further complaints in such a case. Otherwise, if
> + * the commit is invalid, repo_parse_commit() will complain. So
> + * there is nothing for us to say here. Just return failure.
> + */
> + if (repo_parse_commit(r, head_commit))
> + return -1;
Not that I am qualified to do a review of your changes but I am in a
situation where I am trying to understand Git code in general and
(perhaps normal for this situation) wondering about varying styles of
commenting code -- could be that I am just too new to the code base and
do not yet understand the obvious things that don't need comments.
In the above example, there is a short but outstanding comment that
announces a check (and if I understood correctly by [1] it is a kind of
trick that could deserve some more information) and it does _not_
comment on the result. Of course, I have an idea where the correct
place for a comment /* This is an unborn branch -- handle it as if... */
could be, but I'm not sure.
So, my intention is by no means to trigger another spin of this series
-- it is just a view from someone trying to understand not just this
code ;-)
Dirk
[1] https://lore.kernel.org/git/xmqqh6hcu2tg.fsf@gitster.g/
