Glen Choo <chooglen@xxxxxxxxxx> writes: > - <20220304235328.649768-1-jonathantanmy@xxxxxxxxxx> 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. > - <20220304234622.647776-1-jonathantanmy@xxxxxxxxxx> 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 <jonathantanmy@xxxxxxxxxx> [1] https://lore.kernel.org/git/20220304234622.647776-1-jonathantanmy@xxxxxxxxxx/
