On Wed, Apr 13, 2016 at 10:57 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Wed, Apr 13, 2016 at 4:59 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: >> On Wed, Apr 13, 2016 at 10:56 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >>> On Tue, Apr 12, 2016 at 7:02 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: >>>> +test_expect_success '--no-quiet sets quiet to 0' ' >>>> + test-parse-options --no-quiet >output 2>output.err && >>> >>> Meh, as implemented, this isn't a very interesting test, is it? >>> 'quiet' started at 0, so all this shows is that --no-quiet didn't >>> disturb the 0. To really test that it resets it to 0, you'd want: >>> >>> test-parse-options --quiet --no-quiet >... 2>... && >>> >>>> + test_must_be_empty output.err && >>>> + test_cmp expect output >>>> +' >>>> test_done >> >> This is to test whether the 0 of quiet remains 0 if --no-quiet is >> included. This test "defines" the current behavior. Then when I change >> OPT_COUNTUP(), this test will ensure that this behavior is not >> interrupted as promised by the commit message of that patch[1]. I >> guess this also describe why I choose to include these tests between >> 2/5 and 3/5 rather than 3/5 and 4/5. And also see the extended >> discussion[2] for this. If I do a re-roll then I include `--quiet` >> before `--no-quiet` > > Each of these patches should have a single conceptual purpose. It > seems, from the above explanation, that you're mixing and mismatching > bits of such changes between patches. > > * The two new tests for --no-verbose and --no-quiet should be together > and check that they correctly reverse --verbose and --quiet, > respectively. > > * The test you describe above which ensures that --no-quiet leaves > 'quiet' at 0 should be bundled with the change that might break that > behavior, namely, the OPT__COUNTUP() change. I am planning to re-roll this. So, I am just confirming whether I understood properly. * I will add the tests for check for '-q --no-quiet' instead of just '--no-quiet' sets to 0 and '-v --no-verbose' sets to 0 in the patch which improves test coverage which will be before the OPT_COUNTUP() change. * I will then add the test for '--no-quiet' sets to 0 in the separate patch after OPT_COUNTUP() change. Is there something else or something different that you are suggesting? -- 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