Re: [RFC PATCH v1 2/6] stash: remove the second index in stash_working_tree()

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 15, 2020, 12:02 UTC
Message-ID: <CAP8UFD05hdVHsPYPF71xsyU+MOexj7AaSQuezMzy_7GqkOGHMQ@mail.gmail.com>
In-Reply-To: <2158eec0-9332-fe10-3636-95550a66f05d@gmail.com>

On Sat, Jun 13, 2020 at 8:00 PM Alban Gruin <alban.gruin@gmail.com> wrote:

>
> Le 13/06/2020 à 10:52, Christian Couder a écrit :
> > On Tue, May 5, 2020 at 12:56 PM Alban Gruin <alban.gruin@gmail.com> wrote:

Show 18 quoted lines

> >> The call to reset_tree() becomes useless:
> >
> > Your patch doesn't remove any call to reset_tree(), but actually adds
> > one. So the above is difficult to understand.
> >
> > Do you want to say that in a later patch it will be possible to remove
> > the call to reset_tree()? Or do you want to say that the call to
> > write_index_as_tree() becomes useless?
> >
>
> No, I meant that with this commit, reset_tree() does not need to be
> called at the beginning of stash_working_tree(), because it is only
> called by do_create_stash(), which sets the index at `i_tree', and
> save_untracked_files() does not change the main index.  But it will
> become useful again down the line, when save_untracked_file() will be
> rewritten to use the "main" index, so I did not remove it.
>
> I hope it makes more sense now.

Yeah, it makes more sense with an explanation like the above. Instead of "down the line" though, you might want to say something like "in a later commit", as I think it will make it clearer for reviewers that the commit when it will become useful again should be part of the same series.

Thanks, Christian.