Re: [PATCH v5 10/11] setup_git_directory_gently_1(): avoid die()ing

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

Hi Junio,

On Mon, 13 Mar 2017, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The former case may split into two.  "Yes I agree that is bad and it
> > is the same badness as the version without this change", in which case
> > we may want to leave a "NEEDSWORK" comment in-code so that people can
> > notice when browsing the code (but fixing the badness would be outside
> > the scope of the series), and "yes that is bad and we shouldn't
> > introduce that badness", in which case we do want to fix it in the
> > patch.
> 
> We however need to be a bit careful here, though.
> 
> When a patch series is refactoring an existing function to be used
> in different codepaths, an existing issue inherited from the
> original code (e.g. perhaps an existing error checking that is
> looser than ideal) may have been OK in the original context (e.g.
> perhaps it died and refused to run until the user corrected the
> repository), and it still is OK in the codepath that uses the
> refactored building blocks to replace the original code, but it may
> not be acceptable to leave the issue in the original code when a new
> caller calls the refactored building block (e.g. perhaps the
> refactoring made it possible for a caller to ask the function not to
> die and instead act on different kinds of errors in different ways).

In this case, discover_git_directory() is intended to use the exact same
logic as setup_git_directory() (including bugs and all) so that they do
discover the same directory.

It would not do for discover_git_directory() to detect a *different*
directory, no matter how much "more correct" one would deem it.

Ciao,
Johannes