Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-20 Thread Johannes Schindelin
Hi Junio,

On Tue, 19 Jun 2018, Junio C Hamano wrote:

> Todd Zullinger  writes:
> 
> > index e500d7c320..352a52e59d 100755
> > --- a/t/t3404-rebase-interactive.sh
> > +++ b/t/t3404-rebase-interactive.sh
> > @@ -977,7 +977,8 @@ test_expect_success 'rebase -i --root reword root 
> > commit' '
> > set_fake_editor &&
> > FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \
> > git rebase -i --root &&
> > -   git show HEAD^ | grep "A changed"
> > +   git show HEAD^ | grep "A changed" &&
> > +   test -z "$(git show -s --format=%p HEAD^)"
> >  '
> 
> The additional test probably will pass when HEAD is a root commit by
> failing to refer to HEAD^, resulting an empty output from show.  The
> previous step would also give an error and won't emit anything that
> would match "A changed", so it probably is OK, though.

That matches my thinking. Otherwise, we would have had to do the

git show -s --format=%p HEAD^ >out &&
test_must_be_empty out

dance.

Ciao,
Dscho


Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-20 Thread Johannes Schindelin
Hi Junio,

On Tue, 19 Jun 2018, Junio C Hamano wrote:

> Todd Zullinger  writes:
> 
> > With luck, this will save you a few minutes, assuming the
> > commit message is reasonable (or can be improved with help
> > from Phillip and others). :)
> 
> OK.
> 
> > Or Junio may just squash this onto js/rebase-i-root-fix.
> 
> Nah, not for a hotfix on the last couple of days before the final.
> We'd need to build on top, not "squash".

Right. Can we take this on top, at a leisurely pace? I mean: we verified
that this works in the upcoming v2.18.0, and it would be nice to have that
extra regression test safety in the future, but it is not crucial to
include it in v2.18.0 itself.

Ciao,
Dscho


Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-19 Thread Todd Zullinger
Junio C Hamano wrote:
> Todd Zullinger  writes:
>> Or Junio may just squash this onto js/rebase-i-root-fix.
> 
> Nah, not for a hotfix on the last couple of days before the final.
> We'd need to build on top, not "squash".

Indeed.  I somehow missed that you'd merged and pushed the
changes to master and next when I set this.  I was
mistakenly thinking it was only on the js/rebase-i-root-fix
integration branch.

Thanks,

-- 
Todd


Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-19 Thread Junio C Hamano
Todd Zullinger  writes:

> index e500d7c320..352a52e59d 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -977,7 +977,8 @@ test_expect_success 'rebase -i --root reword root commit' 
> '
>   set_fake_editor &&
>   FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \
>   git rebase -i --root &&
> - git show HEAD^ | grep "A changed"
> + git show HEAD^ | grep "A changed" &&
> + test -z "$(git show -s --format=%p HEAD^)"
>  '

The additional test probably will pass when HEAD is a root commit by
failing to refer to HEAD^, resulting an empty output from show.  The
previous step would also give an error and won't emit anything that
would match "A changed", so it probably is OK, though.


Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-19 Thread Junio C Hamano
Todd Zullinger  writes:

> With luck, this will save you a few minutes, assuming the
> commit message is reasonable (or can be improved with help
> from Phillip and others). :)

OK.

> Or Junio may just squash this onto js/rebase-i-root-fix.

Nah, not for a hotfix on the last couple of days before the final.
We'd need to build on top, not "squash".

>
> Thanks.
>
>  t/t3404-rebase-interactive.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index e500d7c320..352a52e59d 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -977,7 +977,8 @@ test_expect_success 'rebase -i --root reword root commit' 
> '
>   set_fake_editor &&
>   FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \
>   git rebase -i --root &&
> - git show HEAD^ | grep "A changed"
> + git show HEAD^ | grep "A changed" &&
> + test -z "$(git show -s --format=%p HEAD^)"
>  '
>  
>  test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on 
> non-interactive rebase' '


Re: [PATCH] t3404: check root commit in 'rebase -i --root reword root commit'

2018-06-19 Thread Johannes Schindelin
Hi Todd,

On Mon, 18 Jun 2018, Todd Zullinger wrote:

> When testing a reworded root commit, ensure that the squash-onto commit
> which is created and amended is still the root commit.
> 
> Suggested-by: Phillip Wood 
> Helped-by: Johannes Schindelin 
> Signed-off-by: Todd Zullinger 

Trusting that this test passes: ACK!

Ciao,
Dscho