Re: [PATCHv2 0/9] Resending sb/submodule-recursive-fetch-gets-the-tip

From: Stefan Beller <sbeller@google.com>
Date: Dec 6, 2018, 21:59 UTC
Message-ID: <CAGZ79kZaw_Pn8mneHYeUebB2vZ+N0FoJBr9NCWp8atfpjLat4g@mail.gmail.com>
In-Reply-To: <xmqqa7lkeki3.fsf@gitster-ct.c.googlers.com>

On Tue, Dec 4, 2018 at 7:10 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

>
> Stefan Beller <sbeller@google.com> writes:
>
> > This is a resend of sb/submodule-recursive-fetch-gets-the-tip,
> > with all feedback addressed. As it took some time, I'll send it
> > without range-diff, but would ask for full review.
>
> Is that a "resend" or reroll/update (or whatever word that does not
> imply "just sending the same thing again")?

As you noticed, it is an actual update. I started to use resend as DScho seems very unhappy about the word reroll claiming we'd be the only Software community that uses the term reroll for an iteration of a change.

I see how resend could sound like retransmission without change.

Show 11 quoted lines

>                         child_process_init(cp);
>      -                  cp->dir = strbuf_detach(&submodule_path, NULL);
>     -                   prepare_submodule_repo_env(&cp->env_array);
>      +                  cp->dir = xstrdup(repo->worktree);
>     +                   prepare_submodule_repo_env(&cp->env_array);
>
> Hmph, I offhand do not see there would be any difference if you
> assigned to cp->dir before or after preparing the repo env, but is
> there a reason these two must be done in this updated order that I
> am missing?  Very similar changes appear multiple times in this
> range-diff.

Jonathan Tan asked for it to be "diff friendly". This -of course- is range-diff unfriendly.

> [...]

you seem to be OK with a lot of the changes, I did not find an actionable suggestion.

Thanks for still queuing topics during -rc time, Stefan