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

From: Guillaume Cogoni <cogoni.guillaume@gmail.com>
Date: Jun 10, 2022, 12:58 UTC
Message-ID: <CAA0Qn1vtPy8GhO-vON9YFPZoN0s2EwwLn3LUUNs1e3N1i5KyBA@mail.gmail.com>
In-Reply-To: <88e41596-d369-958f-f75c-c0b2cd86c2b5@univ-lyon1.fr>

Hello,

Thanks for your reviews. We will take in consideration what you say for the next version.

Sincerely, Guillaume COGONI and Jonathan BRESSAT

On Sat, Jun 4, 2022 at 4:57 PM Matthieu Moy <Matthieu.Moy@univ-lyon1.fr> wrote:

Show 29 quoted lines

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