Hi Junio, On Mon, 13 Mar 2017, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> 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