From: Kristofer Karlsson <krka@spotify.com>
Date: Thu, 08 Oct 2026 06:49:22 GMT
Subject: Re: [PATCH v3 2/2] fetch: write commit-graph using updated refs only
Message-ID: <CAL71e4Mb1FoQn3VqsujAEX_4ziUyrpSuTH+7NkgP9T=QBQQeEw@mail.gmail.com>
In-Reply-To: <ascxqBn0RsXnLWSp@pks.im>

On Thu, 8 Oct 2026 at 08:01, Patrick Steinhardt <ps@pks.im> wrote:
>
> Everything from here...
>
> > After fetch_one() returns, call prepare_commit_graph() (which is
> > made non-static by this commit) to determine the graph-write mode:
> >
> >  - If no commit-graph exists yet, fall back to the full reachable
> >    scan so the first graph creation covers all refs.
> >
> >  - If a commit-graph exists and the fetch updated at least one ref,
> >    write incrementally using only the new refs as seeds.
> >
> >  - If a commit-graph exists but the fetch is a no-op, skip the
> >    commit-graph write entirely.
> >
> >  - For the multi-remote path (fetch --all), where child processes
> >    do the actual fetching, fall back to the full reachable scan.
> >
> > Full commit-graph coverage of all refs remains the responsibility
> > of "git maintenance", "git gc" and "git commit-graph write".
> > Regular Git operations may trigger "git maintenance run --auto",
> > which periodically rebuilds the commit-graph from all reachable
> > refs.
>
> ... to here is still overly verbose, especially the last paragraph. But
> I haven't been complaining about that in the last round, and the rest
> reads significantly better now. So this is not worth another reroll, if
> you ask me.
>
> Other than that I'm happy with this series now, thanks!

I thought the last paragraph was useful for motivating the change,
but I agree it could be written more compactly.

Will change it if I need to reroll anyway, but will keep it as-is
otherwise.

Thanks for reviewing this!
Kristofer