Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled

From: Kevin Daudt <me@ikke.info>
Date: Jun 17, 2015, 10:40 UTC
Message-ID: <20150617104003.GA7345@vps892.directvps.nl>
In-Reply-To: <CACRoPnQ0+h2g2cUJk+8zSkcHjoHjezAGC=5pTjn8aFsgB0GrQA@mail.gmail.com>

On Thu, Jun 11, 2015 at 09:34:08PM +0800, Paul Tan wrote:

Show 6 quoted lines

> On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt <me@ikke.info> wrote:
> > From: Kevin Daudt <compufreak@gmail.com>
> >
> > Signed-off-by: Kevin Daudt <me@ikke.info>
> 
> Ehh? The sign-off does not match the author of the patch.

I changed it, but aparently forgot to reset the author for that commit

Show 7 quoted lines

> 
> >  '
> >
> > +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> > +       test_config branch.to-rebase.rebase true &&
> 
> Ok, though I wonder why not just a git pull --rebase...

Copied that from another test, but was doubting whether to use it or not.

Show 7 quoted lines

> 
> > +       test_config rebase.autostash true &&
> > +       git checkout HEAD -- file &&
> 
> Why not git reset --hard before-rebase? If we don't reset HEAD, then
> how would we know if we actually did a rebase?
>

Good tip, thanks.

> > +       echo dirty > new_file &&
> 
> style: echo dirty >new_file &&
>

Fixed

Show 7 quoted lines

> > +       git add new_file &&
> > +       git pull . copy &&
> > +       test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
> 
> Okay, although it would be better to use "test_cmp_rev HEAD^ copy"
> because it prints out the hashes if they are different.
>

Didn't know about that, and aparently, also not documented. Thanks.

> > +       test $(cat new_file) = dirty &&
> 
> "$(cat new_file)" should be quoted to prevent field splitting.
>

Fixed

New patch is coming.