Re: [PATCH v3 3/3] builtin/commit-graph.c: support '--input=none'
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Feb 13, 2020, 17:58 UTC
- 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:
Show 27 quoted lines
> 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