Re: [PATCH v5 10/11] setup_git_directory_gently_1(): avoid die()ing
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 13, 2017, 21:46 UTC
- Message-ID
- <alpine.DEB.2.20.1703132244400.3767@virtualbox>
- In-Reply-To
- <xmqqvardq66u.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 13 Mar 2017, Junio C Hamano wrote:
Show 23 quoted lines
> 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