Re: [PATCH] git bisect old/new

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 13, 2012, 10:12 UTC
Message-ID: <CAP8UFD1=phEzzwRBmwsLqnbM0_yAS+uMXma_uyqfdq3xo4fTGw@mail.gmail.com>
In-Reply-To: <CABURp0rWu0GtmSrLV+qEQJVYOt8p=B4bQtG3fzKh2f0fTGHwOQ@mail.gmail.com>

On Tue, Jun 12, 2012 at 9:41 PM, Phil Hord <phil.hord@gmail.com> wrote:

Show 21 quoted lines

> On Tue, Jun 12, 2012 at 1:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> >> @@ -403,9 +406,10 @@ struct commit_list *find_bisection(struct
>> >> commit_list *list,
>> >>  static int register_ref(const char *refname, const unsigned char
>> >> *sha1,
>> >>                        int flags, void *cb_data)
>> >>  {
>> >> -       if (!strcmp(refname, "bad")) {
>> >> +       if (!strcmp(refname, bisect_term_bad)) {
>> >>                current_bad_sha1 = sha1;
>> >> -       } else if (!prefixcmp(refname, "good-")) {
>> >> +       } else if (!prefixcmp(refname, "good-") ||
>> >> +                       !prefixcmp(refname, "old-")) {
>> >
>> > I don't like very much "good" and "old" to be hardcoded here.
>>
>> Really?
>
> I tend to agree.  But I like more generic code and less hard-coded in
> almost all cases.

Yeah, I prefer generic code too.

Show 23 quoted lines

>> >> @@ -731,18 +735,25 @@ static void handle_bad_merge_base(void)
>> >>        if (is_expected_rev(current_bad_sha1)) {
>> >>                char *bad_hex = sha1_to_hex(current_bad_sha1);
>> >>                char *good_hex = join_sha1_array_hex(&good_revs, ' ');
>> >> +               if (!strcmp(bisect_term_bad,"bad")) {
>> >> +                       fprintf(stderr, "The merge base %s is bad.\n"
>> >> +                               "This means the bug has been fixed "
>> >> +                               "between %s and [%s].\n",
>> >> +                               bad_hex, bad_hex, good_hex);
>> >> +               } else {
>> >> +                       fprintf(stderr, "The merge base %s is new.\n"
>> >> +                               "The property has changed "
>> >> +                               "between %s and [%s].\n",
>> >> +                               bad_hex, bad_hex, good_hex);
>> >> +               }
>> >
>> > I don't like very much "new" to be harcoded here too.
>>
>> Why not?  It is not like we will be adding any more synonym pair
>> beyond good/bad, so...
>
> Previously we discussed using yes/no, among others.
> http://permalink.gmane.org/gmane.comp.version-control.git/182496

And even if we add "yes/no" maybe people will still want to use "good/bad" instead of "bad/good", "new/old" or "yes/no".

Thanks, Christian.