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

2013-06-09 Thread Thomas Rast
Felipe Contreras felipe.contre...@gmail.com writes:

 +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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-06-09 Thread Felipe Contreras
On Sun, Jun 9, 2013 at 12:22 PM, Thomas Rast tr...@inf.ethz.ch wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

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

Fine.

--- a/builtin/sequencer.c
+++ b/builtin/sequencer.c
@@ -28,9 +28,9 @@ static void finish(struct replay_opts *opts)
if (opts-action != REPLAY_PICK)
return;

-   name = opts-action_name ? opts-action_name : cherry-pick;
+   name = opts-action_name

-   if (!*name)
+   if (!name || !*name)
return;

run_rewrite_hook(rewritten, name);

Now, we won't run when 'git cherry-pick' is called, only when an
action-name is specified; when called from 'git rebase'.

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

No.

http://mid.gmane.org/1370796057-25312-31-git-send-email-felipe.contre...@gmail.com

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

OK.

But it doesn't matter, because the patch won't be applied.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html