Junio C Hamano <gits...@pobox.com> writes: > I _think_ bulk of Antoine and Matthieu's work can be salvaged/reused > to implement the proposal,
I'm obviously biaised since I already spent time on the "bisect terms" idea, and I would hate to see my work and Antoine & Louis' thrown away. But I have to admit that I do like the idea of "git bisect" figuring out which commit is the old and which commit is the new one. It's much easier to implement after the series. I'm currently forbidding redefining "good" as "bad" and vice versa, but that's just to avoid confusion and because I didn't test this case, but it should just work. So, essentially an implementation of this "guess who's old and who's new" could be: if we find a good commit that is an ancestor of an old commit, swap the terms in BISECT_TERMS. When this happens before we started to set any refs, this should do the trick. In general, we should rename the good-$sha1 reference to good and the bad to bad-$sha1 (there are corner-cases where the user already specified several good-$sha1 refs, in which case we would need to discard some of them). I'm getting out of Git time budget, so I can't be the one doing this, at least not soon. So, one option is to take the series as-is, and wait for someone to implement the "guess who's old and who's new" later on top of it. One drawback would be that we'd end up having the not-so-useful feature "bisect terms" in the user-interface. At least, I am now convinced that hardcoding the pair old/new is not needed. In the short term, we can have "git bisect start --name-old foo --name-new bar" which avoids the "One needs to remember in which order to give terms" issue we used to have, so we don't need to clutter the user-interface with many ways to do the same thing. OTOH, the "bisect terms" feature wouldn't be completely useless: not everything is good or bad, so leaving the option to the user to tag "slow/fast", "present/absent", ... still makes sense. So, my proposal would be to remove the "old/new" patch from the series, and keep the other patches. What do you think? > but now it would be more clear that $name_good and $name_bad is a bad > way to name internal variables and files in $GIT_DIR. The inferred 'ah > you are hunting for regression' mode would call old ones 'bad' and new > ones 'good', they have to be given value neutral names, e.g. $name_old > and $name_new. Ideally, the whole code would be ported to use old/new, but the more I read the code the more I agree with Christian actually: we still have many more instances of good/bad in the code (e.g. functions check_good_are_ancestors_of_bad, for_each_good_bisect_ref, ...), so having just name_new/name_old would make the code very inconsistant. It's easier to read the code thinking "good revs are old, bad revs are recent; maybe some magic swapped the terms but I don't need to worry about this" for now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
