Re: [PATCH 11/14] shortlog: allow grouping by committer ident
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jan 4, 2016, 09:44 UTC
- Message-ID
- <CAPig+cT1pNXt2U1bd-KVapiUD=-6TQ8KRBNC8y+0wLAtuqUhQQ@mail.gmail.com>
- In-Reply-To
- <20151229073515.GK8842@sigill.intra.peff.net>
On Tue, Dec 29, 2015 at 2:35 AM, Jeff King <peff@peff.net> wrote:
Show 22 quoted lines
> Shortlog always groups commits by the author field. It can > be interesting to group by other fields, as well. The > obvious other identity in each commit is the committer > field. This might be interesting if your project follows a > workflow where committers and authors are not the same and > where there is not simply one maintainer picking up all of > the patches. > > For instance, you can use this in git.git to see interim and > subsystem maintainers. Likewise, you can see in linux.git > the patches picked up by lieutenants and merged into Linus's > tree. > > This patch also provides some of the necessary > infrastructure for adding more ident types (e.g., from > trailers) in the future. > > Signed-off-by: Jeff King <peff@peff.net> > --- > diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt > @@ -47,6 +47,14 @@ OPTIONS > +--ident=<type>::
Should this be called --group-by?
Show 6 quoted lines
> + By default, `shortlog` collects and collates author identities; > + using `--ident` will collect other types of identity. If > + `<type>` is: > ++ > + - `author`, commits are grouped by author (this is the default) > + - `committer`, commits are grouped by committer
There is a bit of redundancy here. I wonder if it could be described more succinctly. For instance, instead of all the above, perhaps:
Group commits by `<type>`, which can be one of:
- `author` (default)
- `committer`Show 25 quoted lines
> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> index d7eb0cb..39da2d4 100644
> --- a/builtin/shortlog.c
> +++ b/builtin/shortlog.c
> @@ -147,11 +157,23 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
> - format_commit_message(commit, "%an <%ae>", &ident, &ctx);
> - if (ident.len <= 3) {
> - warning(_("Missing author: %s"),
> - oid_to_hex(&commit->object.oid));
> - return;
> + switch (log->ident_type) {
> + case SHORTLOG_IDENT_AUTHOR:
> + format_commit_message(commit, "%an <%ae>", &ident, &ctx);
> + if (ident.len <= 3) {
> + warning(_("Missing author: %s"),
> + oid_to_hex(&commit->object.oid));
> + return;
> + }
> + break;
> + case SHORTLOG_IDENT_COMMITTER:
> + format_commit_message(commit, "%cn <%ce>", &ident, &ctx);
> + if (ident.len <= 3) {
> + warning(_("Missing committer: %s"),
> + oid_to_hex(&commit->object.oid));
> + return;Is this leaking strbuf 'ident'?
(Ditto for the "author" case as mentioned already in the patch 6/14 review.)
> + } > + break; > }