Re: [PATCH v3 0/9] Fix the early config

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Wed, 8 Mar 2017, Jeff King wrote:

> Another "non-gentle" thing I noticed here while looking at
> another thread: the repository-format version check uses the config
> parser, which will die() in certain circumstances. [...]

Yes, that is part of the reason why I was not eager to add that check to
discover_git_directory(). The config code is die()-happy.

This is a much bigger problem, of course, and related to a constant gripe
of mine: I *always* get the quoting wrong in my aliases. Always. And when
I want to fix it, `git config -e` simply errors out because of an invalid
config. Yes, Git, I know, that is the exact reason why I want to edit the
config in the first place.

I am certain you will agree that this is a different topic, therefore
subject to a separate patch series.

In any case, I am fairly certain that the examples you showed demonstrate
that the config has to be rather broken for this patch series to have a
negative impact. And it still would report the broken config so that the
user is not blocked (she can fix the config and call the paginating
command again).

> On Tue, Mar 07, 2017 at 03:31:43PM +0100, Johannes Schindelin wrote:
> 
> > And another change: the GIT_DIR_NONE value was handled incorrectly in
> > discover_git_directory().
> 
> This is the "if (setup_git_directory_1() <= 0)" change from the
> interdiff? That's subtle.

Yes, it is subtle.

> The compiler could have noticed if we used a switch statement here. But
> then any new error conditions would have to be added to that switch
> statement.

We could still do that.

> > I am slightly disappointed that the these additional problems were not
> > spotted in any review but my own. And I had not even included a Duck.
> 
> Get used to being disappointed, I guess. A non-zero number of bugs will
> slip through when writing code _and_ when reviewing it.

I know that. I know that bugs are prone to come in through code
contributions. I don't go for 100%. But I would hope for a better rate
than we have right now: we pride ourselves in the OSS community to make bugs
shallow. I really would like to believe that we catch bugs rather than
discuss formatting (which should be automated) or white-space mangling
(which should not even be a problem if we were truly open for
contributions).

Maybe I am just a grumpy old guy. But I *hate* seeing how much buggy code
we get into Git, despite having a review process that does a very good job
of deterring seasoned developers from contributing.

I really wish it were different. I really wish that we did a better job at
catching bugs before they enter `master`. I really wish that I could be
proud of our code review process.

> > [ceil_offset]
> > Hopefully that clears up the picture?
> 
> Yes, it does. Thanks.

Perfect. Then the time I spent trying to figure all of this out was not
spent in vain.

Ciao,
Dscho