git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Stefan Beller <sbeller@google.com>
Cc: Pratik Karki <predatoramigo@gmail.com>, git <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>,
	Alban Gruin <alban.gruin@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress
Date: Fri, 24 Aug 2018 18:13:24 +0200 (DST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1808241811240.73@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <CAGZ79kafP=dVrwWopXW0WCWf199sU6uFSphjxtMTXDSix4Y81Q@mail.gmail.com>

Hi Stefan,

On Wed, 8 Aug 2018, Stefan Beller wrote:

> 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.

> > +       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

  reply	other threads:[~2018-08-24 16:13 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-08 13:48 [GSoC] [PATCH 00/11] A minimal builtin rebase Pratik Karki
2018-08-08 13:48 ` [PATCH 01/11] builtin rebase: support --onto Pratik Karki
2018-08-08 19:02   ` Junio C Hamano
2018-08-24 16:21     ` Johannes Schindelin
2018-08-08 13:48 ` [PATCH 02/11] builtin rebase: support `git rebase --onto A...B` Pratik Karki
2018-08-08 19:12   ` Junio C Hamano
2018-08-26 18:36     ` Johannes Schindelin
2018-08-08 13:48 ` [PATCH 03/11] builtin rebase: handle the pre-rebase hook (and add --no-verify) Pratik Karki
2018-08-08 19:32   ` Junio C Hamano
2018-08-27 12:15     ` Johannes Schindelin
2018-08-08 13:48 ` [PATCH 04/11] builtin rebase: support --quiet Pratik Karki
2018-08-08 18:31   ` Stefan Beller
2018-08-08 19:37     ` Junio C Hamano
2018-08-27 12:31       ` Johannes Schindelin
2018-08-08 13:48 ` [PATCH 05/11] builtin rebase: support the `verbose` and `diffstat` options Pratik Karki
2018-08-08 13:48 ` [PATCH 06/11] builtin rebase: require a clean worktree Pratik Karki
2018-08-08 13:48 ` [PATCH 07/11] builtin rebase: try to fast forward when possible Pratik Karki
2018-08-08 13:48 ` [PATCH 08/11] builtin rebase: support --force-rebase Pratik Karki
2018-08-08 18:51   ` Stefan Beller
2018-08-24 16:10     ` Johannes Schindelin
2018-08-08 13:48 ` [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress Pratik Karki
2018-08-08 18:59   ` Stefan Beller
2018-08-24 16:13     ` Johannes Schindelin [this message]
2018-08-08 13:48 ` [PATCH 10/11] builtin rebase: only store fully-qualified refs in `options.head_name` Pratik Karki
2018-08-08 13:48 ` [PATCH 11/11] builtin rebase: support `git rebase <upstream> <switch-to>` Pratik Karki
2018-08-08 16:03   ` Duy Nguyen
2018-08-08 18:52     ` Johannes Schindelin
2018-09-04 21:27 ` [PATCH v2 00/11] A minimal builtin rebase Johannes Schindelin via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 01/11] builtin rebase: support --onto Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 02/11] builtin rebase: support `git rebase --onto A...B` Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 03/11] builtin rebase: handle the pre-rebase hook and --no-verify Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 04/11] builtin rebase: support --quiet Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 05/11] builtin rebase: support the `verbose` and `diffstat` options Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 06/11] builtin rebase: require a clean worktree Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 07/11] builtin rebase: try to fast forward when possible Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 08/11] builtin rebase: support --force-rebase Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 09/11] builtin rebase: start a new rebase only if none is in progress Pratik Karki via GitGitGadget
2018-09-04 21:27   ` [PATCH v2 10/11] builtin rebase: only store fully-qualified refs in `options.head_name` Pratik Karki via GitGitGadget
2018-09-08  8:52     ` SZEDER Gábor
2018-09-10 16:55       ` Junio C Hamano
2018-09-10 20:25         ` SZEDER Gábor
2018-09-04 21:27   ` [PATCH v2 11/11] builtin rebase: support `git rebase <upstream> <switch-to>` Pratik Karki via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1808241811240.73@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=alban.gruin@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=predatoramigo@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).