Re: [PATCH v5 0/4] Changed path filter hash fix and version bump
Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> > Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > @@ Documentation/config/commitgraph.txt: commitGraph.maxNewFilters::
> > - If true, then git will use the changed-path Bloom filters in the
> > - commit-graph file (if it exists, and they are present). Defaults to
> > - true. See linkgit:git-commit-graph[1] for more information.
> > -+ Deprecated. Equivalent to changedPathsVersion=1 if true, and
> > ++ Deprecated. Equivalent to changedPathsVersion=-1 if true, and
> > + changedPathsVersion=0 if false.
>
> I forgot to comment on this part earlier, but does the context make
> it clear enough that these `changedPathsVersion` references are
> about `commitGraph.changedPathsVersion` configuration variable
> without fully spelled out? They sit next to each other right now,
> so it may not be too bad. If they appeared across more distance,
> I would be worried, though.
Ah, probably better to spell it out. I'll change it.
Show 10 quoted lines
> > +commitGraph.changedPathsVersion::
> > + Specifies the version of the changed-path Bloom filters that Git will read and
> > -+ write. May be 0 or 1. Any changed-path Bloom filters on disk that do not
> > ++ write. May be -1, 0 or 1. Any changed-path Bloom filters on disk that do not
> > + match the version set in this config variable will be ignored.
>
> So, any time the user configures this to a different value, we will
> start to ignore the existing changed-path-filters data in the
> repository, and when we are told to write commit-graph, we will
> construct changed-path-filters data using the new version?
Show 10 quoted lines
> > -+Defaults to 1.
> > ++Defaults to -1.
> > +++
> > ++If -1, Git will use the version of the changed-path Bloom filters in the
> > ++repository, defaulting to 1 if there are none.
>
> OK, that was misleading. The configuration can say "-1" and it does
> not mean "I'll ignore anything other than version -1"---it means
> "I'll read anything". The earlier statement should be toned down so
> that we do not surprise readers, perhaps
Show 19 quoted lines
> When set to a positive integer value, any changed-path Bloom
> filters on disk whose version is different from the value are
> ignored.
>
> to signal that 0 and negative are special. Then the readers can
> anticipate that special cases are described next.
>
> When set to -1, then ...
> When set to 0, then ...
> Defaults to -1.
>
> When set to the special value -1, what version will we write?
>
> > +If 0, git will write version 1 Bloom filters when instructed to write.
>
> And we will only read 0 and refuse to read 1? Or we will read both
> 0 and 1?
>
> Thanks.
Currently, there is only version 1 (no version 0) and after all the
patches in this patch set are applied, there will be version 1 and
version 2. I think that with your suggestions above, it will be clearer
to the reader.