Re: [PATCH v2 0/2] notes: add notes.merge strategy option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 1, 2015 at 1:12 AM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>
> This series adds a default merge strategy option for git-notes, so that
> the user does not have to type "-s" every time. It is overridden by the
> -s option.

I like this addition. A natural extension (i.e. future work, you
needn't worry about it for now) would be to allow this configuration
to be per notes ref. Different notes refs are used to hold different
kinds of data, and where cat_sort_uniq may be a good strategy for one
particular notes ref, it may be a poor default for other notes refs.
So we should consider adding notes.<notesref>.merge options to allow
more fine-grained control of which notes merge strategies apply to
which notes refs.

> I also added some tests to ensure that the "--abort" "--commit" and "-s"
> options must be independent.

Good. These could easily be split out into a separate commit, though,
as they are independent of the notes.merge addition.

> In addition, I found a small documentation
> bug which is corrected in the first patch of the series.
>
> I Cc'd a couple more people in this version of the patch in order to
> hopefully get some more review.

Thanks, appreciated (although AFAICS the cover letter was not CCed to me).

> This is based on pu incase there were
> any other conflicts, but I can easily rebase if necessary.

Junio has the final word here, but I believe the preferred workflow is
to base your patch series on master or next, as those do not jump
around quite as much as pu does.

>
> Jacob Keller (2):
>   notes: document cat_sort_uniq rewriteMode
>   notes: add notes.merge option to select default strategy

Both patches Acked-by: Johan Herland <johan@xxxxxxxxxxx>


...Johan

>
>  Documentation/config.txt              | 11 +++++--
>  Documentation/git-notes.txt           | 33 +++++++++++++--------
>  builtin/notes.c                       | 55 +++++++++++++++++++++++++----------
>  notes-merge.h                         | 16 +++++-----
>  t/t3309-notes-merge-auto-resolve.sh   | 29 ++++++++++++++++++
>  t/t3310-notes-merge-manual-resolve.sh | 12 ++++++++
>  6 files changed, 119 insertions(+), 37 deletions(-)
>
> --
> 2.5.0.482.gfcd5645
>
> --
> 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



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]