Re: [PATCH v4 0/7] cherry-pick: add `--empty` for more robust redundant commit handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux