Re: Opinions on merging questions (Was: What's cooking in git.git (Feb 2022, #01; Thu, 3))

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 6, 2022, 11:41 UTC
Message-ID: <CAPig+cRrRxuTeByhKkLs_KDaWY8-r4+jrwT83A-r+sBQsmebMw@mail.gmail.com>
In-Reply-To: <20220204081336.3194538-1-newren@gmail.com>

On Fri, Feb 4, 2022 at 7:06 AM Elijah Newren <newren@gmail.com> wrote:

Show 18 quoted lines

> On Fri, Feb 3, 2022 at 21:22 PM Junio C Hamano <gitster@pobox.com> wrote:
> > * ds/sparse-checkout-requires-per-worktree-config (2022-01-31) 5 commits
> >  - worktree: copy sparse-checkout patterns and config on add
> >  - sparse-checkout: set worktree-config correctly
> >  - config: add repo_config_set_worktree_gently()
> >  - worktree: create init_worktree_config()
> >  - Documentation: add extensions.worktreeConfig details
> >
> >  What's the doneness of this one?
> >  source: <pull.1101.v5.git.1643641259.gitgitgadget@gmail.com>
>
> I think it's done and ready for next.
>
> Eric and I weighed in quite a bit on this series, and it changed
> direction pretty dramatically, and more than once.  But we eventually
> all came to an agreement about what should be done (the hard part),
> and this round implements it.  Stolee has diligently fixed or answered
> each item I've raised and I'm very happy with this version.

This version is much improved over earlier versions, and I think everyone is in agreement now that the series is "doing the right thing". However...

I just finished reviewing this round and left a bunch of comments. Some of the comments are minor and wouldn't warrant a reroll, but I also identified some memory leaks and fragile code, as well as non-obvious test code which could be improved. The commit message of [4/5] feels too weak for future readers by not fully explaining the problem(s) that the patch is addressing, so I suggested a possible rewrite of the message. All of these issues could be fixed by follow-on patches (with the exception of the weak commit message), however, taken together it feels like one more reroll is warranted.