Hi Brian
On 25/03/2024 16:12, Brian Lyles wrote:
++ 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"));
Normally we'd write this as
if (!head_name ||
starts_with(head_name, "refs/heads/") ||
!is_null_oid(&head_oid))
return error(...)
breaking lines after an operator and indenting to the open bracket after
the if. The rest looks good. Junio was talking about merging this to
next in the latest "what's cooking" email so I'd double check he hasn't
done that yet before re-rolling.
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 for working on it, I've enjoyed reading your patches
Best Wishes
Phillip