Re: [PATCH 1/3] cherry-pick: add support to copy notes

Felipe Contreras [off-list ref] writes:

Thomas Rast wrote:

quoted

Junio C Hamano [off-list ref] writes:

quoted

Thomas Rast Cc'ed as he has been the primary force behind this line
of "notes" usability.

Thanks for pointing this out to me.

quoted

Felipe Contreras [off-list ref] writes:

quoted

Signed-off-by: Felipe Contreras <redacted>
---
 builtin/revert.c  |   2 +
 sequencer.c       | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 sequencer.h       |   2 +
 t/t3500-cherry.sh |  32 +++++++++++++
 4 files changed, 169 insertions(+), 3 deletions(-)

"git cherry-pick" should help maintaining notes just like amend and
rebase, but how should this interact with notes.rewrite.<command>,
where the command is capable of doing this without an explicit
option once you tell which notes need to be maintained?

Since we already have the notes.rewrite.<command> convention, it would
seem the obvious choice to line it up with the others.  The main
bikeshedding opportunity is whether this should be an exception and
default to false (all other commands default it to true).

Also: how does this interact with notes.rewriteRef and the corresponding
env vars?  Why?

How does it interact with 'cherry-pick -n' if this is done in sequence,
effectively squashing several commits (this use-case is actually
suggested by the manpage), if multiple source commits had notes?  Should
it respect notes.rewriteMode (and by default concatenate)?  (I don't
know if the sequencer state is expressive enough already to carry this
in a meaningful way across cherry-pick commands.)

Feel free to implement that. I'm just interested in 'git cherry-pick' being
usable for 'git rebase' purposes.

Which would have been obvious to all but the most casual readers, eh?

We've been over this already:

  The body should provide a meaningful commit message, which:

    . explains the problem the change tries to solve, iow, what is wrong
      with the current code without the change.

    . justifies the way the change solves the problem, iow, why the
      result with the change is better.

    . alternate solutions considered but discarded, if any.

I'll gladly review your patches again once you have done that.

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

On Wed, May 29, 2013 at 3:09 AM, Thomas Rast [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

Feel free to implement that. I'm just interested in 'git cherry-pick' being
usable for 'git rebase' purposes.

Which would have been obvious to all but the most casual readers, eh?

My motivations are irrelevant, the patch is good as it is.

We've been over this already:

  The body should provide a meaningful commit message, which:

    . explains the problem the change tries to solve, iow, what is wrong
      with the current code without the change.

Obvious; there is no support to copy notes.

    . justifies the way the change solves the problem, iow, why the
      result with the change is better.

Obvious: because it now actually copies the notes.

    . alternate solutions considered but discarded, if any.

There are no alternate solutions.

-- 
Felipe Contreras