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