git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jacob Stopak <jacob@initialcommit.io>
Cc: git@vger.kernel.org, martin.agren@gmail.com
Subject: Re: [RFC PATCH v2] shortlog: add group-by options for year and month
Date: Fri, 23 Sep 2022 09:17:10 -0700	[thread overview]
Message-ID: <xmqqillevzeh.fsf@gitster.g> (raw)
In-Reply-To: <20220922232536.40807-1-jacob@initialcommit.io> (Jacob Stopak's message of "Thu, 22 Sep 2022 16:25:36 -0700")

Jacob Stopak <jacob@initialcommit.io> writes:

> To justify why the new year/month groups should have shorthand flags
> while "--group=trailer:value" does not, I'd say that the fact that
> trailer requires a custom value would make the shorthand version clunky,
> and it wouldn't fit in well with other shorthand options like -n or -s.
> Since year/month have no custom value, the flags make a bit more sense
> and would match up with how "-c, --committer" currently works.

It is an explanation why it is easier to implement and design --year
etc., and does not justify why adding --year etc. is a good idea at
all.  Let's not add the "aliases" in the same patch.

>   - `author`, commits are grouped by author
>   - `committer`, commits are grouped by committer (the same as `-c`)
> + - `month`, commits are grouped by month (the same as `-m`)
> + - `year`, commits are grouped by year (the same as `-y`)

It is unclear what timestamp is used, how a "month" is defined, etc.
As "git shortlog --since=2.years" cuts off based on the committer
timestamp, I would expect that the committer timestamps are used for
this grouping as well?  If I make a commit on the first day of the
month in my timezone, but that instant happens to be still on the
last day of the previous month in your timezone, which month would
your invocation of "git shortlog --group=month" would the commit be
attributed?  My month, or your month?

Does it make sense to even say "group by month and year"?  I expect
that it would mean the same thing as "group by month", and if that
is the case, the command probably should error out or at least warn
if both are given.  An alternative interpretation could be, when
told to "group by month", group the commits made in September 2022
into the same group as the group for commits made in September in
all other years, but I do not know how useful it would be.

Not a suggestion to use a different implementation or add a new
feature on top of this --group-by-time-range idea, but I wonder if
it is a more flexible and generalizeable approach to say "formulate
this value given by the --format=<format> string, apply this regular
expression match, and group by the subexpression value".  E.g.

    git shortlog \
	--group-by-value="%cI" \
	--group-by-regexp="^(\d{4}-\d{2})"

would "formulate the 'committer date in ISO format' value, and apply
the 'grab leading 4 digits followed by a dash followed by 2 digits'
regexp, and group by the matched part".

That's a better way to implement "group by month" internally, and
allow more flexibility.  If a project is well disciplined and its
commit titles follow the "<area>: <description>" convention, you
probably could do

    git shortlog --no-merges \
	--group-by-value="%s" \
	--group-by-regexp="^([^:]+):"

and group by <area> each commit touches.  Of course, existing
--committer and --author can also be internally reimplemented using
the same mechanism.

> @@ -80,6 +82,14 @@ counts both authors and co-authors.
>  --committer::
>  	This is an alias for `--group=committer`.
>  
> +-m::
> +--month::
> +	This is an alias for `--group=month`.
> +
> +-y::
> +--year::
> +	This is an alias for `--group=year`.
> +

Let's not add this in the same patch.  I am fairly negative on
adding more, outside "--group".  Besides, we do not have a good
answer to those who want to group by week.  -w is already taken.

  reply	other threads:[~2022-09-23 16:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-22  6:18 [RFC PATCH] shortlog: add group-by options for year and month Jacob Stopak
2022-09-22 15:46 ` Martin Ågren
2022-09-22 23:25 ` [RFC PATCH v2] " Jacob Stopak
2022-09-23 16:17   ` Junio C Hamano [this message]
2022-09-23 21:19     ` Jacob Stopak
2022-09-23 21:58     ` Jeff King
2022-09-23 22:06       ` Junio C Hamano
2022-09-24  4:38       ` Jacob Stopak
2022-10-05 22:14         ` Jeff King
2022-10-05 21:43       ` Taylor Blau
2022-10-05 22:26         ` Jeff King
2022-10-07  0:48           ` Jacob Stopak
2022-10-07 21:59             ` Taylor Blau
2022-10-11  0:59             ` Jeff King
2022-10-07 22:24           ` Taylor Blau
2022-10-11  1:00             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqillevzeh.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jacob@initialcommit.io \
    --cc=martin.agren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).