Re: [PATCH v2 07/11] commit-graph: write Bloom filters to commit graph file

> Garima Singh <garimasigit@gmail.com> writes:
>> On 2/19/2020 10:13 AM, Jakub Narebski wrote:
>>> "Garima Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:
> [...]
>>>> diff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt
>>>> index a4f17441ae..22e511643d 100644
>>>> --- a/Documentation/technical/commit-graph-format.txt
>>>> +++ b/Documentation/technical/commit-graph-format.txt
>>>> @@ -17,6 +17,9 @@ metadata, including:
>>>>  - The parents of the commit, stored using positional references within
>>>>    the graph file.
>>>>  
>>>> +- The Bloom filter of the commit carrying the paths that were changed between
>>>> +  the commit and its first parent.
>>>> +
>>>
>>> All right.
>>>
>>> Should we also state that it is optional (meta)data?  This would be
>>> first optional piece of data stored in commit-graph, I think.
>>>
>>
>> However the entire commit graph file is non critical metadata since git commands
>> work just fine without it, just slower. The same applies to the changed path
>> bloom filters. 
>>
>> Based on the definition of optional you are suggesting, edge data is optional
>> because not every commit-graph has octopus merges. 
> 
> Well, edge data (EDGE chunk) is optional in different way from Bloom
> filter data.  The former depends on the repository (whether there are
> octopus merges used), the latter is opt-in user choice (whether to run
> `git commit-graph write` with the `--changed-paths` option, or in the
> future equivalent config option).
> 
> To provide some advise that can be acted upon: perhaps it would be
> better to start with "It can store", or end with "if requested" or
> "optionally".  For example the change could look like the following
> suggestion:
> 
> 
>  The Git commit graph stores a list of commit OIDs and some associated
>  metadata, including:
> [...]
> +- The Bloom filter of the commit carrying the paths that were changed between
> +  the commit and its first parent, if requested.
> +
> 
> Best,
>