Re: [PATCH 00/17] [RFC] Commit-graph: Write incremental files

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 8, 2019, 19:27 UTC
Message-ID: <87r298hhlc.fsf@evledraar.gmail.com>
In-Reply-To: <pull.184.git.gitgitgadget@gmail.com>

On Wed, May 08 2019, Derrick Stolee via GitGitGadget wrote:

> This patch series is marked as RFC quality because it is missing some key
> features and tests, but hopefully starts a concrete discussion of how the
> incremental commit-graph writes can work. If this is a good direction, then
> it would replace ds/commit-graph-format-v2.

I have some comments on 12/17 that I'll put there.

I think it would be best to start by submitting 1-11 for inclusion so we can get minor cleanups/refactoring out of the way. I've only skimmed those patches, but they seem to be obviously correct, although the diff move detection (and with -w) doesn't help much with them.

This next bit sounds petty, but I honestly don't mean it that way :)

One minor thing I want to note is 04/17. The change itself I 100% agree on (in-tree docs are bad places for TODO lists), but the commit message *still* says that a "verify" is just as slow as "write", even though I noted a ~40% difference in [1].

Do I care about that tiny isolated thing? Nope. But I *do* think it's indicative of a general thing that could be improved in these RFC iterations that I found a bit frustrating in reading through it. I.e. you're getting some of the "C[comments]", but then there's re-rolled patches that don't address those things.

What we say in the commit message for 4/17 obviously doesn't matter much at all. But there's other outstanding feedback on the last iteration that from reading this one still mostly/entirely applies.

So I'll just leave this reply at "I have a lot of comments", but that they're still sitting there.

1. https://public-inbox.org/git/87o94mql0a.fsf@evledraar.gmail.com/