Re: [PATCH v4 00/23] Fix incorrect use of the_index

From: Elijah Newren <newren@gmail.com>
Date: Jun 9, 2018, 19:58 UTC
Message-ID: <CABPp-BGa+v2x1oeitVDMkQf3qFWx6kUT4wdWhCxjWwPyaW=3bg@mail.gmail.com>
In-Reply-To: <CABPp-BEngspJMEpwoiTq7SCdsQt7fOT_uePS3c6nRxTVqOnNeQ@mail.gmail.com>

On Thu, Jun 7, 2018 at 12:44 AM, Elijah Newren <newren@gmail.com> wrote:

Show 9 quoted lines

> On Wed, Jun 6, 2018 at 9:49 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> v4 fixes some commit messages and killed a couple more the_index
>> references after I tried to merge this with 'pu'
>
> Thanks for tackling this.  The first 8 patches look good to me other
> than the typo I pointed out on patch 4.  However, my eyes glazed over
> a bit on the attr.c stuff in patch 7 that you specifically mentioned
> in the commit message, so my "looks good" doesn't count for as much on
> that one.

I re-read the attr.c stuff in patch 7; looks good to me and the new series makes the code much more readable in the end.

> I'm getting sleepy, but I'll try to circle back and look over the rest
> of the patches in the next few days.

I read over the rest. Found a small grammatical error in a commit message. Found multiple places that still need conversion, from pushing up &the_index usages to callers of ll-merge.c and sha1-file.c instead of having them in those files, to mixes of _cache_ and _index_ functions as in apply.c and merge-recursive.c. However, Duy pointed out there was more work to do, and this series is a step in the right direction and a good start.