From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 13 Feb 2020 17:58:42 GMT
Subject: Re: [PATCH v3 3/3] builtin/commit-graph.c: support '--input=none'
Message-ID: <20200213175842.GE45518@syl.local>
In-Reply-To: <xmqqzhdmsc5s.fsf@gitster-ct.c.googlers.com>

On Thu, Feb 13, 2020 at 08:08:15AM -0800, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
> > On Tue, Feb 11, 2020 at 09:47:57PM -0800, Taylor Blau wrote:
> >> In the previous commit, we introduced '--split=<no-merge|merge-all>',
> >> and alluded to the fact that '--split=merge-all' would be useful for
> >> callers who wish to always trigger a merge of an incremental chain.
> >>
> >> There is a problem with the above approach, which is that there is no
> >> way to specify to the commit-graph builtin that a caller only wants to
> >> include commits already in the graph.
> >
> > I'd like clarification on a detail here.  Is it only about not adding
> > any new commits, or about keeping all existing commits as well?  IOW,
> > do you want to:
> >
> >   - include only commits already existing in the commit-graph, without
> >     adding any new commits, but remove any commits that do not exist
> >     in the object database anymore.
> >
> > or:
> >
> >   - include _all_ commits already existing in the commit-graph, even
> >     those that don't exist anymore in the object database, without
> >     adding any new commits.
>
> FWIW, I read it as the former, but now you brought it up, it can be
> read either way.

It was intended as the former, but I share both of your feelings that it
could be read either way. I amended the commit message to clarify by
adding:

  (and haven't since been deleted from the object store)

as a parenthetical after "already in the graph...".

> Thanks for good review comments, as always.

Yes, indeed: thank very much for your thoughtful feedback.

Thanks,
Taylor