Re: [PATCH v12 2/5] test-parse-options: print quiet as integer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[+cc:Duy Nguyen, Jonathan Nieder]

On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>> Current implementation of parse-options.c treats OPT__QUIET() as integer
>> and not boolean and thus it is more appropriate to print it as integer
>> to avoid confusion.
>
> I can buy this line of reasoning, however, it would be even easier to
> sell the change if you cited an existing client (a git command) which
> actually respects multiple quiet levels. Are there any?

I investigated into this. But I was unable to find any git commit
which actually respects mulitple quiet levels. I first did a 'git grep
OPT__QUIET' to find the commands which use this. Then I went through
the documentation which covers it. None of them have any such mention
of multiple quiet levels. But still I dug further and and went through
all the individual source files. I followed the corresponding C source
code for the header file included and also searched there for any
trace of quiet. But I still didn't find any such use of multiple quiet
levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec,
2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to
say as to why this was introduced and OPT__QUIET which previously used
OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit
In fact git-repack command has quiet but this is not mentioned in the
documentation. If someone could include this it in the documentation.
I would do it but I am not quite familiar with git-repack and haven't
really used it anytime.

> More importantly, though, this change implies that you should also add
> tests to ensure that the quiet level is indeed incremented with each
> --quiet, just as "-vv" and "--verbose --verbose" are already tested.
> You might be able to include such new tests directly in this patch as
> long as the commit message is clear about it, or add them in a
> separate patch.

I will include the tests for multiple level of quiet in the patch.

> By the way, I don't see any tests to ensure that --no-verbose and
> --no-quiet reset those respective values to 0. A separate patch which
> adds such tests would be nice (unless such tests already exist and I
> merely missed them).
There are no tests to ensure that --no-verbose and --no-quiet reset
those respective values to 0 just before this patch. But I have
covered the --no-verbose tests in the upcoming patch 3/5. I will
include the tests for --no-quiet in this patch.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]