Re: [PATCH v7 1/1] Implement rev-list --bisect* --first-parent

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 6, 2019, 02:42 UTC
Message-ID: <xmqqtv7hvi6r.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191105230403.5542-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> Your commit message title should be of the form "<component>: <change>",
> e.g.:
>
>   rev-list: support --first-parent with --bisect*

Good suggestion.

> I would be much more laconic (in particular, omitting subjective terms
> like "minutiae" and "mountains of irrelevant data"), but perhaps that is
> just a matter of subjective style.

FWIW, I had the same reaction. That part of the message was too noisy without adding much actual value.

>> Note, bisecting on --first-parent becomes part of findall's previously
>> existing pass-through as an "option state" flag.
>
> I don't understand this part.

Me neither.

> Also, clarify in the commit message somewhere that this commit does not
> change the behavior of "git bisect".

s/\.$/ when used without the "--first-parent" option&/; you mean?

> As for the diff, besides my comments below, a change in the user-facing
> documentation of "rev-list" is needed, since --bisect and --first-parent
> now work together.

True. I too am, like you are, happy to see that these two options made to work well together.

Thanks, both, for the patch and useful comments. My own review on it may take a bit more time.