Re: [PATCH v4 4/7] sequencer: handle unborn branch with `--allow-empty`

[Dirk Gouders <dirk@gouders.net>]

Brian Lyles <brianmlyles@gmail.com> 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/