Re: [PATCH 0/3] rebase --signoff

2017-04-18 Thread Giuseppe Bilotta
On Tue, Apr 18, 2017 at 2:14 AM, Junio C Hamano  wrote:
> Giuseppe Bilotta  writes:
>
>>>  - How does this interact with "git rebase -i" and other modes of
>>>operation?
>>
>> A better question would maybe be how do we want this to interact?
>
> If "git rebase -i/-m --signoff" will not do anything (which I
> suspect is what we have here), we at least would want it to be
> documented, or the combination be made to error out, I would think.
>
> A better question can wait until that happens ;-)

I've been looking into adding signoff support to the rest of
git-rebase, but the thing is far less trivial to do than I initially
imagined, since the interactive part is split across a number of
sections and files, including C helpers and the sequencer itself. It
can _probably_ be done, but building tests for all corner cases is
quite the daunting task. I think that for the moment I'll resubmit
with the Documentation fix to declare it non-interactive only, and
then leave the extended for later.

-- 
Giuseppe "Oblomov" Bilotta


Re: [PATCH 0/3] rebase --signoff

2017-04-17 Thread Junio C Hamano
Giuseppe Bilotta  writes:

>>  - How does this interact with "git rebase -i" and other modes of
>>operation?
>
> A better question would maybe be how do we want this to interact?

If "git rebase -i/-m --signoff" will not do anything (which I
suspect is what we have here), we at least would want it to be
documented, or the combination be made to error out, I would think.

A better question can wait until that happens ;-)


Re: [PATCH 0/3] rebase --signoff

2017-04-17 Thread Giuseppe Bilotta
On Mon, Apr 17, 2017 at 9:12 AM, Junio C Hamano  wrote:

> Two questions.
>
>  - Is it better to add a brand new test script than adding new tests
>to existing scripts that test "git rebase"?

Since this is a completely (in some sense) new feature, I felt it was
appropriate to put all --signoff-related tests in their own file. So,
if the need arises to put more tests concerning the interaction of
signoff with other stuff, this new test file can be extended.

>  - How does this interact with "git rebase -i" and other modes of
>operation?

A better question would maybe be how do we want this to interact? For
example, with -i: do we want -i --signoff to just sign off everything?
Or do we want a new -i command (o, signoff) to signoff only individual
commits on request? When preserving merges, do we want to sign-off
merge-commits too? I'm not entirely sure what the best policy would
be.

-- 
Giuseppe "Oblomov" Bilotta


Re: [PATCH 0/3] rebase --signoff

2017-04-17 Thread Junio C Hamano
Giuseppe Bilotta  writes:

> Allow signing off a whole patchset by rebasing it with the --signoff
> option, which is simply passed through to git am.

>  Documentation/git-rebase.txt |  5 +
>  builtin/am.c | 39 +
>  git-rebase.sh|  3 ++-
>  t/t3428-rebase-signoff.sh| 46 
> 

Two questions.

 - Is it better to add a brand new test script than adding new tests
   to existing scripts that test "git rebase"?

 - How does this interact with "git rebase -i" and other modes of
   operation?



[PATCH 0/3] rebase --signoff

2017-04-15 Thread Giuseppe Bilotta
Allow signing off a whole patchset by rebasing it with the --signoff
option, which is simply passed through to git am.

Compared to previous incarnations, I've split the am massaging to
separate commits (for cleanliness and easier reverts if needed),
and introduced a test case for both --signoff and its negation.

Giuseppe Bilotta (3):
  builtin/am: obey --signoff also when --rebasing
  builtin/am: fold am_signoff() into am_append_signoff()
  rebase: pass --[no-]signoff option to git am

 Documentation/git-rebase.txt |  5 +
 builtin/am.c | 39 +
 git-rebase.sh|  3 ++-
 t/t3428-rebase-signoff.sh| 46 
 4 files changed, 71 insertions(+), 22 deletions(-)
 create mode 100755 t/t3428-rebase-signoff.sh

-- 
2.12.2.765.g2bf946761b