Re: [PATCH 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

From: Christian Couder <christian.couder@gmail.com>
Date: May 4, 2016, 06:50 UTC
Message-ID: <CAP8UFD1+kEwFhAoveOTYt8NEOK=98W-00nNF+Yoe6kQAYJa6SQ@mail.gmail.com>
In-Reply-To: <CAPig+cQxpZXKqykFoa2kzCZSC2YqpPXnGocs2YttcJ+rCxmzgQ@mail.gmail.com>

On Wed, May 4, 2016 at 8:07 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 5 quoted lines

> On Wed, May 4, 2016 at 1:07 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
>
> This subject is too low-level, talking about implementation details,
> whereas it should be giving a high-level summary of the change.

When a patch is all about a low level detail, I think it is good to talk about the detail in the subject.

Here I think something like "bisect--helper: parse options using OPT_CMDMODE instead of OPT_BOOL" would be ok for example.

Show 7 quoted lines

>> `--next-all` is meant to be used as a sub command to support multiple
>> "operation mode" though the current implementation does not contain any
>> other sub command along side with `--next-all` but further commits will
>> include some more subcommands.
>
> You've spelled this as both "sub command" and "subcommand". Choose one
> and stick with it. ("subcommand" is probably more common.)

I agree with that and the other suggestions, thanks.