Re: sg/commit-graph-cleanups (was Re: What's cooking in git.git (Jul 2020, #01; Mon, 6))

[Derrick Stolee <stolee@gmail.com>]

On 7/7/2020 11:12 AM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> On 7/7/2020 1:57 AM, Junio C Hamano wrote:
>>> * ds/commit-graph-bloom-updates (2020-07-01) 10 commits
>>>   (merged to 'next' on 2020-07-06 at 177e6b362e)
>>>  + commit-graph: check all leading directories in changed path Bloom filters
>>>  + revision: empty pathspecs should not use Bloom filters
>>>  + revision.c: fix whitespace
>>>  + commit-graph: check chunk sizes after writing
>>>  + commit-graph: simplify chunk writes into loop
>>>  + commit-graph: unify the signatures of all write_graph_chunk_*() functions
>>>  + commit-graph: persist existence of changed-paths
>>>  + bloom: fix logic in get_bloom_filter()
>>>  + commit-graph: change test to die on parse, not load
>>>  + commit-graph: place bloom_settings in context
>>>  (this branch uses sg/commit-graph-cleanups.)
>>>
>>>  Updates to the changed-paths bloom filter.
>>>
>>>  Will merge to 'master'.
>>>
>>> * sg/commit-graph-cleanups (2020-06-08) 10 commits
>>>  + commit-graph: simplify write_commit_graph_file() #2
>>>  + commit-graph: simplify write_commit_graph_file() #1
>>>  + commit-graph: simplify parse_commit_graph() #2
>>>  + commit-graph: simplify parse_commit_graph() #1
>>>  + commit-graph: clean up #includes
>>>  + diff.h: drop diff_tree_oid() & friends' return value
>>>  + commit-slab: add a function to deep free entries on the slab
>>>  + commit-graph-format.txt: all multi-byte numbers are in network byte order
>>>  + commit-graph: fix parsing the Chunk Lookup table
>>>  + tree-walk.c: don't match submodule entries for 'submod/anything'
>>>  (this branch is used by ds/commit-graph-bloom-updates.)
>>>
>>>  The changed-path Bloom filter is improved using ideas from an
>>>  independent implementation.
>>>
>>>  Under review.
>>>  cf. <20200627155610.GN2898@szeder.dev>
>>>  cf. <20200627163335.GO2898@szeder.dev>
>>>  cf. <20200627155348.GM2898@szeder.dev>
>>
>> I see these are both in 'next' (thanks!).
>>
>> Perhaps the "Under review" status for sg/commit-graph-cleanups
>> should be modified to match ds/commit-graph-bloom-updates?
> 
> Oops, I am not sure what happened here.  There I thought were issues
> pointed out at least on the latter series but was I hallucinating?

I believe I have resolved all the open comments in both series.

Looking back, I suppose I never responded on the feedback to
"commit-slab: add a function to deep free entries on the slab" that
the deep free is never called. The intention is to add a free
eventually, but at the current point it is not appropriate.

> For now, as -rc0 is so close, I'm tempted to mark both of them to be
> cooked in 'next' during the rc period.

This is completely acceptable. No need to rush this into the RC. I'm
happy to let this cook and do more small improvements on top in the
2.29 cycle, if necessary.

Thanks,
-Stolee