Re: [PATCH v4 20/45] cherry-pick: copy notes and run hooks

From: Thomas Rast <trast@inf.ethz.ch>
Date: Jun 9, 2013, 17:22 UTC
Message-ID: <871u8bdwrb.fsf@hexa.v.cablecom.net>
In-Reply-To: <1370796057-25312-21-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 9 quoted lines

> +static void finish(struct replay_opts *opts)
> +{
> +	if (opts->action != REPLAY_PICK)
> +		return;
> +
> +	run_rewrite_hook(&rewritten, "cherry-pick");
> +	copy_rewrite_notes(&rewritten, "cherry-pick");
> +}
> +

Ok, so I see that with the previous two commits, you automatically get handling of the notes.rewrite.cherry-pick variable and friends. This is good.

However, there are some open points:

* The docs in git-config(1) "notes.rewrite.cherry-pick" and githooks(5)
  "post-rewrite" and are now stale in so far as they contain a list of
  commands doing rewriting.

* This pretends to be cherry-pick even when the hook is called from
  rebase.

  We could claim (and document) that git-rebase with certain options
  shall be the same as running cherry-pick with some other options.
  However, git-am already goes out of its way to ensure that it only
  does the rewriting/post-rewrite if called from rebase (so that the
  user-facing git-am command is not affected).  So it's more consistent
  to ensure that git-cherry-pick, when called from rebase, also pretends
  to be rebase.

* githooks(5) documents explicitly that by the time post-rewrite is
  called, the notes have been rewritten.  Your change does it in the
  opposite order.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch