Glen Choo [off-list ref] writes:

- [ref] I've described the
  differences between the no-submodule-in-index test and the
  other-submodule-in-index test (their comments now refer to one
  another, so the contrast is more obvious), but didn't reorder them
  because I thought that made the test setup less intuitive to read.

Thanks - the comments make sense.

- [ref] I added
  expect.err.sub2 to verify_test_result() but didn't change
  write_expected_super() to account for sub2. It turned out to be tricky
  to predict the output when 'super' fetches >1 branch because each
  fetched branch can affect the formatting. e.g.

    	   OLD_HEAD..super  super           -> origin/super

  can become

    	   OLD_HEAD..super  super                   -> origin/super
    	   OLD_HEAD..super  some-other-branch       -> origin/some-other-branch

  (I could work around this by replacing the whitespace with sed, but it
  seemed like too much overhead for a one-off test).

Overwriting just the super part works for me, thanks.

The only thing remaining from me is my comment about fetching OIDs from
one submodule into another (of the same name but different URL) [1], but
I looked into it myself and we can probably postpone handling this to
another patch set.

In such a patch set, we would probably need to store the URLs that are
reported by upstream .gitmodules somewhere. (I forgot that we don't use
them in this patch.) And then, either implement an autosync function
(like "git submodule sync", perhaps gated by a "--sync-submodules"
argument so that users can include it when fetching new commits and
exclude it when fetching historical commits) and/or use those URLs in a
diagnostic message to be printed when the fetch fails.

As it is, the existing fetch-into-submodules-at-HEAD also suffers from
the same flaw, so I'm OK postponing this to another patch set.

So,
Reviewed-by: Jonathan Tan <redacted>

[1] https://lore.kernel.org/git/20220304234622.647776-1-jonathantanmy@google.com/ (local)