On Tue, Aug 11, 2015 at 10:57 PM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote: > From: Jacob Keller <jacob.keller@xxxxxxxxx> > > Add new option "notes.<ref>.merge" option which specifies the merge > strategy for merging into a given notes ref. 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 > 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. Thanks for working on this, and I apologize for not properly reviewing this part of the series before now. More comments below. > > Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx> > --- > Documentation/config.txt | 6 ++++++ > Documentation/git-notes.txt | 6 ++++++ > builtin/notes.c | 7 +++++-- > t/t3309-notes-merge-auto-resolve.sh | 39 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 56 insertions(+), 2 deletions(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 488c2e8eec1b..2c283ebc309e 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1912,6 +1912,12 @@ notes.merge:: > STRATEGIES" section of linkgit:git-notes[1] for more information > on each strategy. > > +notes.<localref>.merge:: > + Which merge strategy to choose if the local ref for a notes merge > + matches <localref>, overriding "notes.merge". <localref> just be a s/just/must/ > + fully qualified refname. See "NOTES MERGE STRATEGIES" section in > + linkgit:git-notes[1] for more information on the available strategies. I sort of get the reason for "fully qualified refname", but I think notes.refs/notes/commits.merge looks much uglier than notes.commits.merge Especially since we have the opposite precedence for branch.<name>.*, e.g. we already have branch.master.merge and not branch.refs/heads/master.merge I know that we don't yet have a "proper" place to put remote notes refs, but the <ref> in notes.<ref>.merge _must_ be a "local" notes ref (you even use the <localref> notation in the documentation below). Thus, I believe we can presume that the local notes ref must live under refs/notes/*, and hence drop the "refs/notes/" prefix from the config key (just like branch.<name>.* can presume that <name> lives under refs/heads/*). ... Which brings me to another small gripe about the naming here: branch.<name>.merge names the remote ref (at branch.<name>.remote) that we will pull from, but notes.<ref>.merge has a very different meaning. If we - in the future - were to provide a similar config mechanism for a hypothetical "git notes pull" command, then it would be natural to model its config similarly: notes.<name>.remote and notes.<name>.merge specifies whence we fetch, and what we (notes-)merge, respectively. Except that this patch in its current form will occupy the .merge config key... Can you rename to notes.<name>.mergestrategy instead? Or even better, take inspiration from branch.<name>.mergeoptions, and provide notes.<name>.mergeoptions instead, which you can then set like: git config notes.foo.mergeoptions "--strategy=cat_sort_uniq" Even though 'git notes merge' don't yet accept any other options that should be configurable, I think it's worth emulating the mechanisms that alread exist for branches. It gives is some amount of future- proofing as well. ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html