Re: [PATCH 2/3] merge-base: return fork-point outside reflog

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2017, 01:49 UTC
Message-ID: <xmqqzi9nlcyw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <5a2fca1d-4edf-965f-4840-58c924c91051@grubix.eu>

Michael J Gruber <git@grubix.eu> writes:

Show 6 quoted lines

> Also, I'm undecided about about your reflog argument above - if we leave
> "--fork-point" to be the current behaviour including Jeff's fix then the
> documentation would need an even bigger overhaul, because it's neither
> "reflog also" (as claimed in the doc) nor "reflog only" (as in the
> original implementation) but "historical tips as inferred from the
> current value and the reflog".

Even though things like "reflog only", "reflog also", may be something implementors may care about, they are irrelevant implementation details to the intended audience. "The bases that are not in reflogs are ignored" _does_ matter, as it affects the outcome, and that may be a bit too strict a filter (which this series takes us in a good direction to fix) but what the readers need to know is the real-world implications of the choices made at the implementation detail level, and more importantly, what the implementation is trying to compute.

It is a documentation bug (with or without these patches) if the current text gives an impression that the code is trying to do anything but "guessing the fork point using historical tips".

> In any case, for two modes we need two names for the options. Maybe
> --fork-point and --fork-base because in the loose mode, you may get a
> "base of a strict fork point"?

Perhaps.