Hi Phillip
On Mon, Mar 25, 2024 at 9:38 AM <phillip.wood123@xxxxxxxxx> wrote:
> @@ sequencer.c: static struct object_id *get_cache_tree_oid(struct index_state *ist
> - */
> - if (repo_parse_commit(r, head_commit))
> - return -1;
> ++ const char *head_name;
> ++
> + if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
> + /*
> -+ * Treat an error reading HEAD as an unborn branch.
> ++ * Check to see if this is an unborn branch
> + */
>
> You are only editing an existing comment here and it is not worth a
> re-roll on its own, but for one line comments we prefer
>
> /* Check to see if this is an unborn branch */
Existing in v3, but new with this series. Since it sounds like a re-roll
is desired anyway to address the line length issue below, I can go ahead
and fix this in v5 as well.
> ++ 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))
>
> While we don't mind the occasional line that is a little over 80
> characters these really are rather long.
>
You're right, these got a little long. I wasn't able to identify a
definitive wrapping style for these cases, so I'll include my proposed
update here just to avoid another re-roll. Does the following diff from
v4 to a proposed v5 work for you?
@@ -776,11 +776,13 @@ static int is_index_unchanged(struct repository *r)
const char *head_name;
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))
+ /* 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 {
> Apart from the minor style issues this all looks good to me, thanks
> for working on it, it will be a useful addition to be able to drop
> cherry-picks that become empty.
Thanks, I really appreciate your help with this series!
--
Thank you,
Brian Lyles
