Re: What's cooking in git.git (Mar 2018, #03; Wed, 14)

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 16, 2018, 22:08 UTC
Message-ID: <20180316220824.GC151588@aiede.svl.corp.google.com>
In-Reply-To: <xmqqh8pfzo1h.fsf@gitster-ct.c.googlers.com>

Hi,

Junio C Hamano wrote:

> Jonathan Tan <jonathantanmy@google.com> writes:
>> On Wed, 14 Mar 2018 18:34:49 -0700
>> Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

>>> * sb/object-store (2018-03-05) 27 commits
>>
>> [snip list of commits]
>>
>>>  (this branch is used by sb/packfiles-in-repository; uses nd/remove-ignore-env-field.)
>>>
>>>  Refactoring the internal global data structure to make it possible
>>>  to open multiple repositories, work with and then close them.
>>>
>>>  Rerolled by Duy on top of a separate preliminary clean-up topic.
>>>  The resulting structure of the topics looked very sensible.
>>>
>>>  Waiting for a follow-up discussion.
>>
>> Would it be possible for this set to go in independently of
>> nd/remove-ignore-env-field? I understand that some patches might be
>> cleaner if ignore_env is first removed, but this patch set has already
>> undergone several rounds of review and (I think) is an improvement to
>> the codebase on its own.
>
> I thought the "remove-ignore-env-field" thing is a quite small and
> more-or-less straightforward improvements that would serve as a good
> preparatory change to give a solid foundation to the object-store
> topic.
>
> I was hoping to hear quick Acks for remove-ignore-env (and also
> Duy's reroll of their topics on it) from people involved in all the
> related topics, so that we can advance them more-or-less at the same
> time.

I can go along with this for this series, but I want to explain why I am not thrilled with the process in general.

The series "Moving global state into the repository object (part 1)" had gone through three revisions with relatively minor changes between each and we were in the middle of reviewing the fourth. In response to a series that comes after it, "(part 2)", Duy created nd/remove-ignore-env, a series that I quite like.

But with this reroll, the result is:

 * The patches that have already been reviewed 3 times (in their
   current incarnation; parts of this series have also appeared on
   list earlier too) and we were in the middle of reviewing a fourth
   time are now on a new base.

 * These three series, each of a manageable size, have been combined
   into a larger series of 44 patches.

 * I have fears about when they're ever going to land and be part of
   an API I can start making use of.

So even though I agree with the goal and I like the improved initialization code, I am not happy.

I would be happier with just the new initialization code being its own series that we can fast-track, then deal with part 1 separately on top, and then deal with part 2 after that.

Thanks, Jonathan