Re: [PATCH v3 00/38] Teach the sequencer to act as rebase -i's backend

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 16, 2017, 10:45 UTC
Message-ID: <alpine.DEB.2.20.1701161129240.3469@virtualbox>
In-Reply-To: <xmqqinpgdass.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Sat, 14 Jan 2017, Junio C Hamano wrote:

Show 20 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > You stated elsewhere that converting a script into a builtin should focus
> > on a faithful conversion.
> >
> > The original code is:
> >
> > 	. "$author_script"
> 
> [...]
>
> If the code in the sequencer.c reads things other than the three
> variables we ourselves set, and make them into environment variables
> and propagate to subprocesses (hooks and editors), it would be a
> bug.  The original did not intend to do that (the dot-sourcing is
> overly loose than reading three known variables and nothing else,
> but is OK because we do not support the case where end users muck
> with the file).  Also, writing FOO=BAR alone (not "export FOO=BAR"
> or "FOO=BAR; export FOO") to the file wouldn't have exported FOO to
> subprocesses anyway.

That analysis cannot be completely correct, as the GIT_AUTHOR_* variables *are* used by the `git commit` subprocess.

In any case, it is clear that we (I include all reviewers here) messed this patch series up quite a bit. Hence I will be more careful from now on to only act on suggestions that do, in fact, improve the patch series from my point of view.

Ciao, Johannes