On Mon, Dec 10, 2018 at 8:09 PM Yaroslav Halchenko
<deb...@onerussian.com> wrote:
Thanks for the patches. The first patch looks good to me!
> [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status
> --recursive to stay intact
The subject is a bit cryptic (specifically the first part before the
colon), maybe
t7406: compare entire submodule status for --reset-hard mode
?
> For submodule update --reset-hard the best test is comparison of the
> entire status as shown by submodule status --recursive. Upon update
> --reset-hard we should get back to the original state, with all the
> branches being the same (no detached HEAD) and commits identical to
> original (so no merges, new commits, etc).
"original state" can mean different things to different people. I'd think
we could be more precise:
... we should get to the state that the submodule is reset to the
object id as the superprojects gitlink points at, irrespective of the
submodule branch.
> test_expect_success 'submodule update --merge staying on master' '
> (cd super/submodule &&
> - git reset --hard HEAD~1
> + git reset --hard HEAD~1
unrelated white space change?
> ) &&
> (cd super &&
> (cd submodule &&
> @@ -307,16 +318,28 @@ test_expect_success 'submodule update --merge staying
> on master' '
> '
>
> test_expect_success 'submodule update --reset-hard staying on master' '
> [..]
> +'
> +
The tests look good to me, though I wonder if we'd rather want to inline
{record/compare}_submodule_status as then you'd not need to look it up
and the functions are rather short?
