On Fri, Oct 19, 2018 at 6:20 PM Jeff King <peff@xxxxxxxx> wrote:
>
> On Mon, Sep 03, 2018 at 08:09:27PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
> > diff --git a/submodule.c b/submodule.c
> > index 50cbf5f13e..c0c1224760 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -766,7 +766,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
> > * have a corresponding 'struct oid_array' (in the 'util' field) which lists
> > * what the submodule pointers were updated to during the change.
> > */
> > -static void collect_changed_submodules(struct string_list *changed,
> > +static void collect_changed_submodules(struct index_state *istate,
> > + struct string_list *changed,
> > struct argv_array *argv)
> > {
> > struct rev_info rev;
>
> This function doesn't actually make use of the newly added "istate" (nor
> does it use the_index, so there's not a spot that forgot to get
> converted). Is this in preparation for more refactoring, or is it just a
> mistake and can be dropped?
It's possible that it was needed at some point when I converted diff
api to pass index_state around. But in later iterations I think I
passed "struct repository *" instead because diff code needed much
more than the index, but did not clean this up. Sorry.
In this function, we still have the_repository for
repo_init_revisions() and it should be gone eventually. "struct
repository *r" could take the place of "struct index_state *istate"
instead and we would need to reopen the path again.
So I'm not sure, if it's really bad, we could remove it now. Otherwise
we could just leave it there, I don't think this "istate" will survive
very long. I already started deleting some of the_repository in "kill
the_index" series.
> I don't see anything in the "Kill the_index, final part" series that
> would need it.
Yeah. Killing the_index is just the first small step (didn't look that
small when I started). Now it's all about the_repository ;-) and we
have like 400 references of it just in library code.
> If this can be dropped, then most of the other changes here can, too,
> because they're just pushing the unused parameter down the stack. I.e.:
--
Duy
