Re: [WIP PATCH 1/3] git bisect old/new

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 4, 2015, 18:05 UTC
Message-ID: <xmqqh9qnuz6z.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433404753-12039-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr>

Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> writes:

Show 10 quoted lines

> From: Christian Couder <chriscool@tuxfamily.org>
>
> When not looking for a regression during a bisect but for a fix or a
> change in another given property, it can be confusing to use 'good'
> and 'bad'.
>
> This patch introduce `git bisect new` and `git bisect old` as an
> alternative to 'bad' and good': the commits which have the most
> recent version of the property must be marked as `new` and the ones
> with the older version as `old`.

The phrase "the most recent version of" invites an unnecessary confusion. "git bisect" is a tool to find a single bit flip and if the behaviour changed twice in the history from A to B to C, you cannot look for the transition between A to B or B to C. You need to say "I classify behaviour B and C to be both good (new) and I know it used to have behaviour A which was bad (old)".

    You can mark the commits with the "new" property as "new", and
    the "old" property as "old"; "good" being "new" and "bad" being
    "old" becomes a mere special case of this.  The "new" property
    in the traditional bisection is "commit has the bug we are
    hunting", while "old" is "commit lacks that bug".

Show 17 quoted lines

> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> index 4cb52a7..12c7711 100644
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
> ...
> @@ -104,6 +104,33 @@ For example, `git bisect reset HEAD` will leave you on the current
>  bisection commit and avoid switching commits at all, while `git bisect
>  reset bisect/bad` will check out the first bad revision.
>  
> +
> +Alternative terms: bisect new and bisect old
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If you are not at ease with the terms "bad" and "good", perhaps
> +because you are looking for the commit that introduced a fix, you can
> +alternatively use "new" and "old" instead.
> +But note that you cannot mix "bad" and good" with "new" and "old".

There is a dq missing somewhere on this line.

Show 6 quoted lines

> +
> +------------------------------------------------
> +git bisect new [<rev>]
> +------------------------------------------------
> +
> +Marks the commit as new, which means the version is fixed.

I am not sure ", which means the version is fixed." is a good idea.

The whole point of introducing "old" vs "new", the reason why we may want to do this patch series, is to reduce confusion by removing the value judgement associated with "good" vs "bad". That is what made the bisect confusing when the old ones were bad and new ones were good. Saying "fixed" implicitly contrasts it with "broken", and introduces the value judgement on the states again.

	Side note: yes, if the reader is careful, "fixed" needs to
	have previous states to be "broken", so in that sense,
	"fixed" can be read about time-sequence and not about value
	judgement.  But let's try to be understandable by even
	casual readers.

We would instead want to emphasize that what we are judging is not values but time sequence. You would want to say "It used to be X but it no longer is X", without saying if X is a good thing or not, e.g.

	Marks the commit as new, e.g. "the bug is no longer there",
        if you are looking for a commit that fixed a bug, or "the
        feature that used to work is now broken at this point", if
        you are looking for a commit that introduced a bug.

Show 5 quoted lines

> +------------------------------------------------
> +git bisect old [<rev>...]
> +------------------------------------------------
> +
> +Marks the commit as old, which means the bug is present.

This is much worse than the previous one. Unless you force the reader to _assume_ that you are hunting for a fix, "bug is present" can mean "bug is still present", or "bug was introduced somewhere along the line and now it is there". And a casual mention of "perhaps because you are looking for a fix" at the beginning of the section is not strong enough hint that these examples are _only_ about "hunting for bugfix".

I'd stop here for now, but a casual skimming told me that the same issue exists throughout the remainder of the doc.

Thanks.