From: Johan Herland <johan@herland.net>
Date: Mon, 17 Aug 2015 13:21:09 GMT
Subject: Re: [PATCH v8 8/8] notes: teach git-notes about notes.<ref>.mergeStrategy option
Message-ID: <CALKQrgeBVfkWOPZYJLnLqJiBPG5XR++G2Mmma2tyc-kwZ5aW_A@mail.gmail.com>
In-Reply-To: <1439801191-3026-9-git-send-email-jacob.e.keller@intel.com>

Allow me to suggest a different wording, somewhat inspired by the
branch.<name>.* documentation...

On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> Add new option "notes.<ref>.mergeStrategy" option which specifies the merge
> strategy for merging into a given notes ref.

Add new "notes.<name>.mergeStrategy" config, which specifies the merge
strategy for notes merges into refs/notes/<name>.

> This option enables
> selection of merge strategy for particular notes refs, rather than all
> notes ref merges, as user may not want cat_sort_uniq for all refs, but
> only some. Note that the <ref> is the local reference we are merging

s/<ref>/<name>/

> into, not the remote ref we merged from. The assumption is that users
> will mostly want to configure separate local ref merge strategies rather
> than strategies depending on which remote ref they merge from. Also,
> notes.<ref>.merge overrides the general behavior as it is more specific.

same here

>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>  Documentation/config.txt            |  7 +++++++
>  Documentation/git-notes.txt         |  6 ++++++
>  builtin/notes.c                     | 14 ++++++++++++-
>  t/t3309-notes-merge-auto-resolve.sh | 39 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 56e20446f587..a48c111d3ce0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1926,6 +1926,13 @@ notes.mergeStrategy::
>         STRATEGIES" section of linkgit:git-notes[1] for more information
>         on each strategy.
>
> +notes.<localref>.mergeStrategy::
> +       Which merge strategy to choose if the local ref for a notes merge
> +       matches <localref>, overriding "notes.mergeStrategy". <localref> must
> +       be the short name of a ref under refs/notes/.

notes.<name>.mergeStrategy::
        Which merge strategy to use when doing a notes merge into
        refs/notes/<name>. This overrides the more general
"notes.mergeStrategy".

Otherwise, the series (except possibly #4/#5, see separate discussion)
looks good to me.


...Johan



-- 
Johan Herland, <johan@herland.net>
www.herland.net