Re: [PATCH 1/5] merge-recursive: find-renames resets threshold

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 21, 2016, 17:18 UTC
Message-ID: <CAPig+cQa0t3DuXEP=zW1=2bSMro632c7DfMHcLo_iN9xhGqLqg@mail.gmail.com>
In-Reply-To: <1456067358-19781-2-git-send-email-felipegassis@gmail.com>

On Sun, Feb 21, 2016 at 10:09 AM, Felipe Gonçalves Assis <felipeg.assis@gmail.com> wrote:

> Make the find-renames option follow the behaviour in git-diff, where it
> resets the threshold when none is given. So, for instance,
> "--find-renames=25 --find-renames" should result in the default
> threshold (50%) instead of 25%.

Makes sense. I'd have expected to see the corresponding tests bundled along with this patch rather than included in a separate patch (5/5) whose title doesn't indicate any direct relation to this change (although the corresponding test is mentioned briefly in the 5/5 commit message).

You might consider re-ordering the patches so that the patch (3/5) which adds missing tests for existing features instead comes first. Then, bundle the tests for this "fix" directly into this patch.

Show 24 quoted lines

> Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
> ---
>  merge-recursive.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 7bff5be..b880ae5 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -2094,8 +2094,10 @@ int parse_merge_opt(struct merge_options *o, const char *s)
>                 o->renormalize = 0;
>         else if (!strcmp(s, "no-renames"))
>                 o->detect_rename = 0;
> -       else if (!strcmp(s, "find-renames"))
> +       else if (!strcmp(s, "find-renames")) {
>                 o->detect_rename = 1;
> +               o->rename_score = 0;
> +       }
>         else if (skip_prefix(s, "find-renames=", &arg) ||
>                  skip_prefix(s, "rename-threshold=", &arg)) {
>                 if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
> --
> 2.7.1.492.gc9722f8
>