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

[phillip.wood123@gmail.com]

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