Re: [PATCH 2/8] shortlog: refactor committer/author grouping
- From
Jeff King <peff@peff.net>
- Date
- Sep 27, 2020, 07:59 UTC
- Message-ID
- <20200927075927.GA1286220@coredump.intra.peff.net>
- In-Reply-To
- <CAN0heSoS837ZCLt8WLRBrthXgOErwz-vXW8Af9v2ZdCcCpWRiw@mail.gmail.com>
On Sat, Sep 26, 2020 at 02:31:32PM +0200, Martin Ågren wrote:
Show 13 quoted lines
> On Fri, 25 Sep 2020 at 09:04, Jeff King <peff@peff.net> wrote: > > > > In preparation for adding more grouping types, let's > > refactor the committer/author grouping code. In particular: > > > > - the master option is now "--group", to make it clear > > that the various group types are mutually exclusive. The > > "--committer" option is an alias for "--group=committer". > > I think this is more than a refactoring of the code. The user interface > is also refactored (if that's even the right word). From the subject and > the first sentence above, I did not expect this first "-" item, nor that > the patch would touch Documentation/.
Yeah, I agree it's a bit misleading. I've reworded it (especially the title) to make the intent more clear.
Show 8 quoted lines
> > + enum {
> > + SHORTLOG_GROUP_AUTHOR = 0,
> > + SHORTLOG_GROUP_COMMITTER
> > + } group;
>
> You could reduce the patch noise by adding a trailing comma, see
> cc0c42975a ("CodingGuidelines: spell out post-C89 rules", 2019-07-16).
> (I do realize that you later redefine all values anyway.)Thanks, that's worth doing. This part of the series was actually from 2015, so perhaps before we had that guideline. :)
-Peff