On Tue, Jun 06, 2023 at 09:01:10AM +0200, Patrick Steinhardt wrote: > On Mon, May 15, 2023 at 03:23:07PM -0400, Taylor Blau wrote: > > Here is a reroll of my series to implement jump (née skip) lists for the > > packed refs backend. > > > > Not a ton has changed since last time, but some notable things that have > > changed include: > > > > - Renaming "skip lists" to "jump lists" to clarify that this > > implementation does not use the skip list data structure. > > - Patch reorganization, splitting out `find_reference_location_end()` > > more sensibly, rewording patch messages, etc. > > - Addresses feedback from Junio and Patrick Steinhardt's helpful > > reviews. > > > > As usual, a range-diff is included below for convenience. > > > > Given that we are expecting -rc0 today, we should aim to not let review > > of this topic direct our attention away from testing the release > > candidates. We can get more serious about it on the other side of 2.41. > > > > Thanks in advance for another look. > > I didn't have many comments in this round. Personally though I'd split > up this patch series into two in order to land the individual parts > faster, where the first part introduces `git for-each-ref --exclude` and > the second part introduces the jump list for the packed-refs backend. Thanks for reviewing, and for the good suggestion. I think that splitting this series in two could be worthwhile, but I'm not sure I want to make this change for v4. You could imagine splitting the series there, where the first half would implement the naive version of `for-each-ref --exclude`, and the second half would implement jump lists, and make upload- and receive-pack take advantage of it. But I think that makes the first half trivial and leaves all of the complexity in this series to the latter half. I suppose that makes it easier to graduate the first six or so patches earlier, but they aren't really all that useful without the remaining patches. Another split might be: - the naive implementation of `for-each-ref --exclude` - jump lists, and using them within `for-each-ref` - teaching upload- and receive-pack to use the jump list But juggling this series as three sub-topics feels like it would be burdensome to the maintainer ;-). So I think I'd rather leave it as-is, unless you feel strongly. Thanks, Taylor
