public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Jacob Keller <jacob.e.keller@intel•com>
Cc: git@vger•kernel.org, Johan Herland <johan@herland•net>,
	Eric Sunshine <sunshine@sunshineco•com>,
	Jacob Keller <jacob.keller@gmail•com>
Subject: Re: [PATCH v9 3/6] note: extract parse_notes_merge_strategy to notes-utils
Date: Mon, 17 Aug 2015 15:38:12 -0700	[thread overview]
Message-ID: <xmqqbne5bkx7.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1439847214-2980-4-git-send-email-jacob.e.keller@intel.com> (Jacob Keller's message of "Mon, 17 Aug 2015 14:33:31 -0700")

Jacob Keller <jacob.e.keller@intel•com> writes:

> From: Jacob Keller <jacob.keller@gmail•com>
>
> Combining rewrite and notes-merge functionality has been left as an
> exercise for a future contributor.

This comment is probably unnecessary; we haven't even established if
such a combination is desirable AFAICT in the discussion.

The patch itself looks very straightforward and obviously good.

Thanks.

>
> Signed-off-by: Jacob Keller <jacob.keller@gmail•com>
> ---
>  builtin/notes.c | 12 +-----------
>  notes-utils.c   | 18 ++++++++++++++++++
>  notes-utils.h   |  1 +
>  3 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 042348082709..0e7aba98f4d7 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -797,17 +797,7 @@ static int merge(int argc, const char **argv, const char *prefix)
>  	o.remote_ref = remote_ref.buf;
>  
>  	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_merge_strategy(strategy, &o.strategy)) {
>  			error("Unknown -s/--strategy: %s", strategy);
>  			usage_with_options(git_notes_merge_usage, options);
>  		}
> diff --git a/notes-utils.c b/notes-utils.c
> index ccbf0737a34e..299e34bccc58 100644
> --- a/notes-utils.c
> +++ b/notes-utils.c
> @@ -54,6 +54,24 @@ void commit_notes(struct notes_tree *t, const char *msg)
>  	strbuf_release(&buf);
>  }
>  
> +int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s)
> +{
> +	if (!strcmp(v, "manual"))
> +		*s = NOTES_MERGE_RESOLVE_MANUAL;
> +	else if (!strcmp(v, "ours"))
> +		*s = NOTES_MERGE_RESOLVE_OURS;
> +	else if (!strcmp(v, "theirs"))
> +		*s = NOTES_MERGE_RESOLVE_THEIRS;
> +	else if (!strcmp(v, "union"))
> +		*s = NOTES_MERGE_RESOLVE_UNION;
> +	else if (!strcmp(v, "cat_sort_uniq"))
> +		*s = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
> +	else
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static combine_notes_fn parse_combine_notes_fn(const char *v)
>  {
>  	if (!strcasecmp(v, "overwrite"))
> diff --git a/notes-utils.h b/notes-utils.h
> index db5811e3f718..fa538e1d9502 100644
> --- a/notes-utils.h
> +++ b/notes-utils.h
> @@ -37,6 +37,7 @@ struct notes_rewrite_cfg {
>  	int mode_from_env;
>  };
>  
> +int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s);
>  struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
>  int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
>  			  const unsigned char *from_obj, const unsigned char *to_obj);

  reply	other threads:[~2015-08-17 22:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-17 21:33 [PATCH v9 0/6] implement notes.mergeStrategy Jacob Keller
2015-08-17 21:33 ` [PATCH v9 1/6] notes: document cat_sort_uniq rewriteMode Jacob Keller
2015-08-17 21:33 ` [PATCH v9 2/6] notes: extract enum notes_merge_strategy to notes-utils.h Jacob Keller
2015-08-17 21:33 ` [PATCH v9 3/6] note: extract parse_notes_merge_strategy to notes-utils Jacob Keller
2015-08-17 22:38   ` Junio C Hamano [this message]
2015-08-18  0:15     ` Jacob Keller
2015-08-17 21:33 ` [PATCH v9 4/6] notes: add tests for --commit/--abort/--strategy exclusivity Jacob Keller
2015-08-17 21:33 ` [PATCH v9 5/6] notes: add notes.mergeStrategy option to select default strategy Jacob Keller
2015-08-17 21:33 ` [PATCH v9 6/6] notes: teach git-notes about notes.<name>.mergeStrategy option Jacob Keller
2015-08-17 22:35 ` [PATCH v9 0/6] implement notes.mergeStrategy Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbne5bkx7.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jacob.e.keller@intel$(echo .)com \
    --cc=jacob.keller@gmail$(echo .)com \
    --cc=johan@herland$(echo .)net \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox