Re: [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 24, 2018, 16:13 UTC
Message-ID: <nycvar.QRO.7.76.6.1808241811240.73@tvgsbejvaqbjf.bet>
In-Reply-To: <CAGZ79kafP=dVrwWopXW0WCWf199sU6uFSphjxtMTXDSix4Y81Q@mail.gmail.com>

Hi Stefan,

On Wed, 8 Aug 2018, Stefan Beller wrote:

Show 13 quoted lines

> On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki <predatoramigo@gmail.com> wrote:
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 8a7bf3d468..a261f552f1 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >                 usage_with_options(builtin_rebase_usage,
> >                                    builtin_rebase_options);
> >
> > +       /* Make sure no rebase is in progress */
> 
> The faithful conversion doesn't even stop at the comments. ;-)

Yes, I insisted on it.

TBH it is a bit of a shame that we cannot fix all those error messages going to stdout, but... you know... One step after the other.

> I shortly wondered if this is the best place for this comment,
> but let's just keep it here to have the 1:1 rewrite.

It should probably be inside the conditional block, but as you say: the original had it in a funny spot, and so does the converted code.

Show 6 quoted lines

> > +       if (in_progress) {
> [...]
> > +                   state_dir_base, cmd_live_rebase,buf.buf);
> 
> In case a resend is needed, add a whitespace after the
> comma and buf.buf, please.

I will fix this before sending the next iteration.

Thanks for the review! Dscho