From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 14 Aug 2015 18:07:03 GMT
Subject: Re: [PATCH v5 3/4] notes: add notes.mergestrategy option to select default strategy
Message-ID: <CAPig+cQN9bszBiW4dxp8cz+aJUf=70V-vLQio_6OX9ry1cOYRw@mail.gmail.com>
In-Reply-To: <1439574620-25249-4-git-send-email-jacob.e.keller@intel.com>

On Fri, Aug 14, 2015 at 1:50 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> Teach git-notes about "notes.mergestrategy" to select a general strategy
> for all notes merges. This enables a user to always get expected merge
> strategy such as "cat_sort_uniq" without having to pass the "-s" option
> manually.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 042348082709..97109f8d419c 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -796,21 +814,16 @@ static int merge(int argc, const char **argv, const char *prefix)
>         expand_notes_ref(&remote_ref);
>         o.remote_ref = remote_ref.buf;
>
> +       git_config_get_string_const("notes.mergestrategy", &configured_strategy);
> +
>         if (strategy) {
> -               if (!strcmp(strategy, "manual"))
> -                       o.strategy = NOTES_MERGE_RESOLVE_MANUAL;
> -               else if (!strcmp(strategy, "ours"))
> -                       o.strategy = NOTES_MERGE_RESOLVE_OURS;
> -               else if (!strcmp(strategy, "theirs"))
> -                       o.strategy = NOTES_MERGE_RESOLVE_THEIRS;
> -               else if (!strcmp(strategy, "union"))
> -                       o.strategy = NOTES_MERGE_RESOLVE_UNION;
> -               else if (!strcmp(strategy, "cat_sort_uniq"))
> -                       o.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
> -               else {
> +               if (parse_notes_strategy(strategy, &o.strategy)) {
>                         error("Unknown -s/--strategy: %s", strategy);

This error message points the user at the source of the problem
("--strategy"). Good.

>                         usage_with_options(git_notes_merge_usage, options);
>                 }
> +       } else if (configured_strategy) {
> +               if (parse_notes_strategy(configured_strategy, &o.strategy))
> +                       die("Unknown notes merge strategy: %s", configured_strategy);

This error message doesn't. Perhaps it should mention the config
variable "notes.mergestrategy" to help the user track down the problem
more quickly?

>         }
>
>         t = init_notes_check("merge");