From: Toon Claes <toon@iotcl•com>
To: Taylor Blau <me@ttaylorr•com>
Cc: git@vger•kernel.org,
Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail•com>,
Derrick Stolee <stolee@gmail•com>,
Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCH v5 0/6] Introduce git-last-modified(1) command
Date: Tue, 22 Jul 2025 17:35:34 +0200 [thread overview]
Message-ID: <87a54wkzll.fsf@iotcl.com> (raw)
In-Reply-To: <aHmJmwvYmAyi3Nlh@nand.local>
Taylor Blau <me@ttaylorr•com> writes:
> On Wed, Jul 16, 2025 at 03:32:00PM +0200, Toon Claes wrote:
>> This series adds the git-last-modified(1) to feed this view. In the past
>> the subcommand was proposed[1] to be named git-blame-tree(1). This
>> version is based on the patches shared by the kind people at GitHub[2].
>
> Sorry for completely dropping this from my review queue. Let me try and
> give it a read...
No worries, we all got work to do ;)
>> What is different from the series shared by GitHub:
>>
>> * Renamed the subcommand from `blame-tree` to `last-modified`. There was
>> some consensus[5] this name works better, so let's give it a try and
>> see how this name feels.
>
> Hmmph. I prefer the "blame-tree" name personally, but I am (a) biased,
> and (b) used to it over "last-modified", so I don't think my preference
> or bias should count for much here.
Well, for what it's worth I like "blame-tree" more as well. But didn't
feel strong enough to push it through.
>> * Patches for --max-depth are excluded. I think it's a separate topic to
>> discuss and I'm not sure it needs to be part of series anyway. The
>> main patch was submitted in the previous attempt[3] and if people
>> consider it valuable, I'm happy to discuss that in a separate patch
>> series.
>
> Yeah, makes sense.
I might be revisiting this, because recently I've integrating the WIP
command in our tech stack I noticed having this option would be
useful/required.
>> * The last-modified command isn't recursive by default. If you want
>> recurse into subtrees, you need to pass `-r`.
>
> OK.
>
>> * The patches in 'tb/blame-tree' at Taylor's fork[4] implements a
>> caching layer. This feature reads/writes cached results in
>> `.git/blame-tree/<hash>.btc`. To keep this series to a reviewable
>> size, that feature is excluded from this series. I think it's better
>> to submit this as a separate series.
>
> Makes sense; the caching feature was primarily implemented by Stolee and
> I think for our purposes here can be considered additive and not
> essential to the basic functionality of this new command. For what it's
> worth, I *would* like[^1] to see those features sent to the list at some
> point, but I agree that they are a significant source of additional
> complexity. So punting on them for now seems like the right direction to
> me.
>
> [^1]: My ulterior motive here would be to eventually ditch GitHub's
> "blame-tree" command entirely and remove it from GitHub's diff to
> upstream. I'm happy to help however I can with that effort once this
> series lands.
Obviously. I know how it goes, more code maintained by the community is
better for everyone.
>> * Squashed various commits together. Like they introduced a flag
>> `--go-faster`, which later became the default and only implementation.
>> That story was wrapped up in a single commit.
>
> Perfect, thank you. I figured that we would not want to keep temporary
> measures around like the "--go-faster" flag, but I also figured that
> they may be helpful in unpacking the history of this command, hence why
> I sent them in the first place.
I wasn't sure about this. I've had a hard time unraveling what in the 55
patches in your branch was valuable and what could be squashed into
other commits.
>> * Dropped the patches that attempt to increase performance for tree
>> entries that have not been updated in a long time. In my testing I've
>> seen both performance improvements *and* degradation with these
>> changes:
>>
>> Test HEAD~ HEAD
>> ------------------------------------------------------------------------------------
>> 8020.1: top-level last-modified 4.52(4.38+0.11) 2.03(1.93+0.08) -55.1%
>> 8020.2: top-level recursive last-modified 5.79(5.64+0.11) 8.34(8.17+0.11) +44.0%
>> 8020.3: subdir last-modified 0.15(0.09+0.06) 0.19(0.14+0.06) +26.7%
>>
>> Before we include these patches, I want to make sure these changes
>> have positive impact in all/most scenarios. This can happen in a
>> separate series.
>
> Hmm. It's been long enough that I honestly don't remember the details
> here, but I agree that this is worth looking into at some point in the
> future.
I've had this patch included in version 2[1]. I'd love to include it,
but it didn't give the results we were expecting. Over time I became
more confortable with these changes. Let me see if I can get more
insights about it.
>> I've set myself as the author and added Based-on-patch-by trailers to
>> credit the original authors. Let me know if you disagree.
>
> I can't speak for the other authors of this command, but I have no issue
> being ~~blamed~~ credited with a "Based-on-patch-by" trailer ;-).
>
>> Again thanks to Taylor and the people at GitHub for sharing these
>> patches. I hope we can work together to get this upstreamed.
>
> Ditto.
<3
[1]: https://lore.kernel.org/git/20250523-toon-new-blame-tree-v2-4-101e4ca4c1c9@iotcl.com/
--
Cheers,
Toon
next prev parent reply other threads:[~2025-07-22 15:35 UTC|newest]
Thread overview: 135+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 17:46 [PATCH RFC 0/5] Introduce git-blame-tree(1) command Toon Claes
2025-04-22 17:46 ` [PATCH RFC 1/5] blame-tree: introduce new subcommand to blame files Toon Claes
2025-04-24 16:19 ` Junio C Hamano
2025-05-07 13:13 ` Toon Claes
2025-04-22 17:46 ` [PATCH RFC 2/5] t/perf: add blame-tree perf script Toon Claes
2025-04-22 17:46 ` [PATCH RFC 3/5] blame-tree: use Bloom filters when available Toon Claes
2025-04-22 17:46 ` [PATCH RFC 4/5] blame-tree: implement faster algorithm Toon Claes
2025-04-22 17:46 ` [PATCH RFC 5/5] blame-tree.c: initialize revision machinery without walk Toon Claes
2025-04-23 13:26 ` [PATCH RFC 0/5] Introduce git-blame-tree(1) command Marc Branchaud
2025-05-07 14:22 ` Toon Claes
2025-05-07 20:23 ` Marc Branchaud
2025-05-07 20:45 ` Junio C Hamano
2025-05-08 13:26 ` Marc Branchaud
2025-05-08 14:26 ` Junio C Hamano
2025-05-08 15:12 ` Marc Branchaud
2025-05-14 14:42 ` Toon Claes
2025-05-14 19:29 ` Junio C Hamano
2025-05-14 21:15 ` Marc Branchaud
2025-05-15 13:29 ` Patrick Steinhardt
2025-05-15 16:39 ` Junio C Hamano
2025-05-15 17:39 ` Marc Branchaud
2025-05-15 19:30 ` Jeff King
2025-05-16 4:38 ` Patrick Steinhardt
2025-05-20 8:49 ` Toon Claes
2025-05-15 17:30 ` Marc Branchaud
2025-05-16 4:30 ` Patrick Steinhardt
2025-05-14 21:15 ` Marc Branchaud
2025-05-07 20:49 ` Kristoffer Haugsbakk
2025-05-08 13:20 ` D. Ben Knoble
2025-05-08 13:26 ` Marc Branchaud
2025-05-08 13:18 ` D. Ben Knoble
2025-05-23 9:33 ` [PATCH RFC v2 0/5] Introduce git-last-modified(1) command Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 1/5] last-modified: new subcommand to show when files were last modified Toon Claes
2025-05-25 20:07 ` Justin Tobler
2025-06-05 8:32 ` Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-06-13 9:34 ` Toon Claes
2025-06-13 9:52 ` Kristoffer Haugsbakk
2025-05-23 9:33 ` [PATCH RFC v2 2/5] t/perf: add last-modified perf script Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 3/5] last-modified: use Bloom filters when available Toon Claes
2025-05-27 10:40 ` Patrick Steinhardt
2025-06-13 11:05 ` Toon Claes
2025-05-23 9:33 ` [PATCH RFC v2 4/5] last-modified: implement faster algorithm Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-05-23 9:33 ` [PATCH RFC v2 5/5] last-modified: initialize revision machinery without walk Toon Claes
2025-05-27 10:39 ` Patrick Steinhardt
2025-07-01 20:35 ` [PATCH RFC v2 0/5] Introduce git-last-modified(1) command Kristoffer Haugsbakk
2025-07-01 21:06 ` Junio C Hamano
2025-07-01 21:30 ` Kristoffer Haugsbakk
2025-07-02 13:00 ` Toon Claes
2025-07-09 15:53 ` Toon Claes
2025-07-09 17:00 ` Junio C Hamano
2025-06-30 18:49 ` [PATCH RFC v3 0/3] " Toon Claes
2025-06-30 18:49 ` [PATCH RFC v3 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-01 20:20 ` Kristoffer Haugsbakk
2025-07-02 11:51 ` Junio C Hamano
2025-06-30 18:49 ` [PATCH RFC v3 2/3] t/perf: add last-modified perf script Toon Claes
2025-06-30 18:49 ` [PATCH RFC v3 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-01 23:01 ` [PATCH RFC v3 0/3] Introduce git-last-modified(1) command Junio C Hamano
2025-07-09 15:26 ` [PATCH v4 " Toon Claes
2025-07-09 21:57 ` Junio C Hamano
2025-07-10 18:37 ` Junio C Hamano
2025-07-16 13:32 ` [PATCH v5 0/6] " Toon Claes
2025-07-16 13:35 ` [PATCH v5 1/6] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-18 0:02 ` Taylor Blau
2025-07-19 6:44 ` Jeff King
2025-07-22 15:50 ` Toon Claes
2025-08-01 9:09 ` Christian Couder
2025-08-01 16:59 ` Junio C Hamano
2025-07-16 13:35 ` [PATCH v5 2/6] t/perf: add last-modified perf script Toon Claes
2025-07-18 0:08 ` Taylor Blau
2025-07-22 15:52 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 3/6] last-modified: use Bloom filters when available Toon Claes
2025-07-18 0:16 ` Taylor Blau
2025-07-22 16:02 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 4/6] pretty: allow caller to disable indentation Toon Claes
2025-07-16 15:50 ` Junio C Hamano
2025-07-17 16:31 ` Toon Claes
2025-07-16 13:35 ` [PATCH v5 5/6] last-modified: support --extended format Toon Claes
2025-07-16 16:09 ` Junio C Hamano
2025-07-17 16:31 ` Toon Claes
2025-07-17 22:37 ` Junio C Hamano
2025-07-18 17:36 ` Junio C Hamano
2025-07-22 16:06 ` Toon Claes
2025-07-16 13:42 ` [PATCH v5 6/6] fixup! last-modified: use Bloom filters when available Toon Claes
2025-07-17 23:39 ` [PATCH v5 0/6] Introduce git-last-modified(1) command Taylor Blau
2025-07-22 15:35 ` Toon Claes [this message]
2025-07-30 17:59 ` Toon Claes
2025-07-31 7:45 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 0/4] " Toon Claes
2025-07-31 18:40 ` Junio C Hamano
2025-07-31 23:57 ` Junio C Hamano
2025-08-05 9:33 ` [PATCH v7 0/3] " Toon Claes
2025-08-05 14:34 ` Patrick Steinhardt
2025-08-05 16:21 ` Junio C Hamano
2025-08-05 16:34 ` Junio C Hamano
2025-08-05 16:55 ` Toon Claes
2025-08-05 17:20 ` Jean-Noël AVILA
2025-08-05 21:46 ` Junio C Hamano
2025-08-06 12:01 ` Toon Claes
2025-08-06 15:38 ` Junio C Hamano
2025-08-28 22:44 ` Junio C Hamano
2025-08-05 18:28 ` Junio C Hamano
2025-08-05 9:33 ` [PATCH v7 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-08-05 9:33 ` [PATCH v7 2/3] t/perf: add last-modified perf script Toon Claes
2025-08-05 9:33 ` [PATCH v7 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-30 17:55 ` [PATCH v6 1/4] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-31 6:42 ` Patrick Steinhardt
2025-08-01 16:22 ` Toon Claes
2025-08-01 17:09 ` Junio C Hamano
2025-08-04 6:34 ` Patrick Steinhardt
2025-08-04 17:14 ` Junio C Hamano
2025-08-05 5:35 ` Toon Claes
2025-08-01 20:34 ` Jean-Noël AVILA
2025-08-05 5:36 ` Toon Claes
2025-08-04 6:33 ` Patrick Steinhardt
2025-08-01 10:18 ` Christian Couder
2025-08-01 10:22 ` Patrick Steinhardt
2025-08-01 17:06 ` Junio C Hamano
2025-08-02 8:18 ` Christian Couder
2025-08-02 11:31 ` Christian Couder
2025-08-02 13:38 ` Christian Couder
2025-08-02 16:26 ` Junio C Hamano
2025-08-04 6:35 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 2/4] t/perf: add last-modified perf script Toon Claes
2025-07-30 17:55 ` [PATCH v6 3/4] commit-graph: export prepare_commit_graph() Toon Claes
2025-07-31 6:42 ` Patrick Steinhardt
2025-07-30 17:55 ` [PATCH v6 4/4] last-modified: use Bloom filters when available Toon Claes
2025-07-31 6:43 ` Patrick Steinhardt
2025-08-01 16:23 ` Toon Claes
2025-08-04 6:33 ` Patrick Steinhardt
2025-07-09 15:26 ` [PATCH v4 1/3] last-modified: new subcommand to show when files were last modified Toon Claes
2025-07-09 15:26 ` [PATCH v4 2/3] t/perf: add last-modified perf script Toon Claes
2025-07-09 15:26 ` [PATCH v4 3/3] last-modified: use Bloom filters when available Toon Claes
2025-07-16 13:35 ` [PATCH v5 6/6] fixup! " Toon Claes
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a54wkzll.fsf@iotcl.com \
--to=toon@iotcl$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=stolee@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox