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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antoine Delaite <antoine.delaite@xxxxxxxxxxxxxxxxxxxxxxx> writes:

> From: Christian Couder <chriscool@xxxxxxxxxxxxx>
>
> 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".

> 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.

> +
> +------------------------------------------------
> +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.

> +------------------------------------------------
> +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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]