Re: [PATCH v2 2/4] config: fix config scope enum

From: Matt Rogers <mattr94@gmail.com>
Date: Jan 9, 2020, 23:29 UTC
Message-ID: <CAOjrSZu4PRR17fjqJNK6r61OKJHchR7nkgcX3VBzLEZHb+BUBA@mail.gmail.com>
In-Reply-To: <xmqqpnfszbyh.fsf@gitster-ct.c.googlers.com>

On Thu, Jan 9, 2020 at 2:06 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>
> "Matthew Rogers via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Matthew Rogers <mattr94@gmail.com>
> >
> > Previously when iterating through git config variables, worktree config
> > and local config were both considered "CONFIG_SCOPE_REPO".  This was
> > never a problem before as no one had needed to differentiate between the
> > two cases.
>
> Hmph, then "fix" on the title is a bit misleading, no?
>
> The enum may not have been as fine grained as you would have liked,
> but if there was nothing broken, then we are doing this not to "fix"
> anything.
>

I see where you're coming from, but I would definitely consider this a "fix" in that it's something that (as explained in the deleted comment) should have been this way from the start but was unnecessary as no one had a need for it yet. But I definitely wouldn't be against changing the phrasing to something like "clean up" or whatever your preferred wording would be.

Show 9 quoted lines

> A more important thing to explain would probably be why we
> (i.e. those who propose this change, those who give favoriable
> reviews to it, and those who accept it change to the system) would
> want to see finer-grained classification.  What do we expect to be
> able to do with the resulting finer-grained set that we wouldn't be
> able to with the original, and why is it a good thing?  That is what
> readers of the proposed log message of this change would want to
> see, I would think.
>

This is really more prep for patch 4 later on in this series, as a user who ran the proposed '--show-scope' later on in the series would care what was "worktree" vs "local".

Regardless, I think the two options I have would be to amend the commit message with that extra information or roll it together with patch 4