Re: [PATCH v2 4/4] error message now advice to use the new option

From: Matthieu Moy <matthieu.moy@univ-lyon1.fr>
Date: Jun 4, 2022, 09:45 UTC
Message-ID: <88e41596-d369-958f-f75c-c0b2cd86c2b5@univ-lyon1.fr>
In-Reply-To: <82beb916d9c44a069f30ec4ff261e3be@SAMBXP02.univ-lyon1.fr>

On 5/27/22 21:55, Jonathan Bressat wrote:
 > Subject: Re: [PATCH v2 4/4] error message now advice to use the new 
option

Commit messages are usually written with imperative tone. The "now" doesn't add information, the reader can guess that the commit message describes the new behavior.

"suggest --overwrite-same-content in error message when appropriate" ?

Show 7 quoted lines

> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -760,6 +760,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   				       &new_branch_info->commit->object.oid :
>   				       &new_branch_info->oid, NULL);
>   		topts.preserve_ignored = !opts->overwrite_ignore;
> +		topts.overwrite_same_content = 0;/* FIXME: opts->overwrite_same_content */

Why not use opts->overwrite_same_content in the code rather than saying you should in a comment?

Actually, doesn't this hunk belong to the previous commit?

The rest of the patch looks good to me.

-- 
Matthieu Moy
https://matthieu-moy.fr/