On Wed, Apr 27, 2016 at 11:25 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Mon, Apr 25, 2016 at 2:40 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: >> On Wed, Apr 13, 2016 at 10:57 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >>> 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. > > These tests would be even a bit more interesting if you tested "-q -q > --no-quiet" and "-v -v --no-verbose", respectively, to ensure that the > "no" options actually reset to 0 rather than merely decrementing by 1. This seems a better choice. > I recall also suggesting adding a new test checking that "-q -q" > increments the quiet count to 2 (which could be done in the patch > which makes 'quiet' print as a number instead of a boolean or in the > same "improve test coverage" patch). Will include this. >> * I will then add the test for '--no-quiet' sets to 0 in the separate >> patch after OPT_COUNTUP() change. > > No, this and "--no-verbose sets to 0" are logically related to the > OPT__COUNTUP() change, thus would be incorporated into that patch. > Alternately, these two tests could just be part of "improve test > coverage" patch, establishing base behavior which the OPT__COUNTUP() > patch shouldn't break. I would prefer including it in "improve test coverage" patch to establish the base behavior. This seems more natural to me. You can expect this series from me within 2 days. Thanks. -- 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