From: Matthieu Moy <matthieu.moy@univ-lyon1.fr>
Date: Sat, 04 Jun 2022 09:45:13 GMT
Subject: Re: [PATCH v2 4/4] error message now advice to use the new option
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" ?

> --- 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/