Glen Choo <chooglen@xxxxxxxxxx> writes:
> IMO "&& remote" is non-inuitive enough to warrant a comment, e.g.
>
> /* do not update submodules if fetch_multiple() was called */
> if (!result && remote && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
As I am not so familiar with this codepath when submodules are
involved, I need to be explained why having called fetch_multiple()
means there is no need to update submodules".
/*
* This is only needed after fetch_one(), which does not
* fetch submodules by itself.
*
* fetch_multiple() has already updated submodules to grab
* commits necessary for the fetched history from each remote,
* so there is no need to fetch submodules from here.
*/
perhaps?
> The test looks good, but I think it belongs in
> t/t5526-fetch-submodules.sh. I don't see anything else in
> t5617-clone-submodules-remote.sh that calls "git fetch".
Good eyes.
> In addition, I think we need one more test that adds another remote. In
> the above test, we only have one remote, so we hit your new optimization
> and already pass the test without the need for "&& remote".
True, too.
