Re: [PATCH v2 0/6] Let the sequencer handle `git rebase -i --root`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 5, 2018, 19:24 UTC
Message-ID: <nycvar.QRO.7.76.6.1805052121090.77@tvgsbejvaqbjf.bet>
In-Reply-To: <CAGZ79ka2SmfA_ZUA2MjuqSwb=gUphbRxp=NQgj5AcqvMiePZFw@mail.gmail.com>

Hi Stefan,

On Fri, 4 May 2018, Stefan Beller wrote:

Show 11 quoted lines

> > Branch-diff vs v1:
> >  1: 42db734a980 ! 1: 73398da7119 sequencer: learn about the special "fake root commit" handling
> >      @@ -54,40 +54,50 @@
> >         return NULL;
> >        }
> >
> >      ++/* Read author-script and return an ident line (author <email> timestamp) */
> >       +static const char *read_author_ident(struct strbuf *buf)
> 
> 
> I like the new way of read_author_ident. Thanks for writing it!

You're welcome. After sleeping a night over it, I think this function might also benefit from a new name: extract_ident_from_author_script(). What do you think?

Show 18 quoted lines

> >      @@ -159,7 +169,17 @@
> >       +/* Does this command create a (non-merge) commit? */
> >       +static int is_pick_or_similar(enum todo_command command)
> >       +{
> >      -+ return command <= TODO_SQUASH;
> >      ++ switch (command) {
> >      ++ case TODO_PICK:
> >      ++ case TODO_REVERT:
> >      ++ case TODO_EDIT:
> >      ++ case TODO_REWORD:
> >      ++ case TODO_FIXUP:
> >      ++ case TODO_SQUASH:
> >      ++         return 1;
> >      ++ default:
> >      ++         return 0;
> >      ++ }
> 
> The switch case is not as bad as I thought following the discussion on of v1.

Yes, it makes things explicit, and it is not too long a case-chain.

> This series is
> Reviewed-by: Stefan Beller <sbeller@google.com>

Thanks!

Show 5 quoted lines

> <off topic>
> During a lunch discussion I wondered if the branch diff format could lead to
> another form of machine readable communication, i.e. if we want to add the
> ability to read the branch diff format and apply the changes. Note how applying
> this diff-diff would not create new commits, but rather amend existing commits.

</off topic> (which BTW is not valid XML)

I do not think that it would be a wise idea to detour even further from Git when exchanging patch series iterations. We have a lovely way to exchange commits, after all: `git fetch` and `git push`, and for times you cannot agree on a central server, `git bundle`.

Ciao, Dscho