From: Bagas Sanjaya Date: Wed, 28 Apr 2021 04:09:20 GMT Subject: Re: [PATCH] bisect--helper: use BISECT_TERMS in 'bisect skip' command Message-ID: In-Reply-To: On 26/04/21 14.06, Christian Couder wrote: > Thanks Bagas for your test! I will take a look at it soon. > > My opinion is that it would be best if both patches (Ramsay's and > Bagas') were in the same patch series or even perhaps in the same > commit. If you prefer separate patches, maybe the first one could be > Ramsay's, and the second one Bagas' where indeed the instructions to > replace test_expect_failure with test_expect_success have been > followed. > OK. Review ping > It might not be the best API for this (or the set_terms() and > get_terms() function could perhaps have better names), but anyway the > current situation is that set_terms(&terms, "bad", "good") is setting > the current terms to "bad"/"good" which is the default, and then > get_terms(&terms) is reading the terms stored in the BISECT_TERMS file > and using that to set the current terms. Also if the BISECT_TERMS file > doesn't exist, then get_terms(&terms) is doing nothing. So it seems to > me that Ramsay's patch is doing the right thing. > > If get_terms(&terms) was used before set_terms(&terms, "bad", "good"), > then the current terms would always be "bad"/"good" even if the > BISECT_TERMS file contains valid terms different from "bad"/"good". > OK, thanks for the explanation. -- An old man doll... just what I always wanted! - Clara