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