Re: [PATCH] submodule: Alllow staged changes for get_superproject_working_tree

[Sam McKelvie <sammck@gmail.com>]

> On Sep 27, 2018, at 6:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Sam McKelvie <sammck@gmail.com> writes:
> 
>>>> Subject: Re: [PATCH] submodule: Alllow staged changes for get_superproject_working_tree
>>> 
>>> s/Alllow/allow/;
>>> 
>> 
>> Ok, no caps on first letter of subject.
> 
> Ah, that, too.  I meant to correct triple ell, though ;-)
> 
> When one reviewer says Reviewed-by: but you later found that the
> patch was not good enough to deserve a redoing, feel free to redo
> the patch and do not add the Reviewed-by: you got for the old one
> to your second submission.  The difference between the one that was
> reviewed and the one you updated invalidates the stale Reviewed-by:,
> essentially.
> 
> Some reviewers explicitly state "With this and that nit corrected or
> left as-is, the patch is Reviewed-by: me" to tell you that as long
> as the difference between the version reviewed and the updated one
> is limited within the named issues, you can add Reviewed-by: to your
> rerolled patch.
> 
> In this case, I think the nits are pretty small and I do not mind
> tweaking the version we are discussing on my end, without having you
> to send an updated one.
> 
> Here is what I'd squash into your commit, with title corrected, if
> you are OK with that plan.  In the meantime, I'll keep the following
> as a separate patch on top when I merge your fix to 'pu'.
> 
> Thanks.

I wholeheartedly approve of that plan and your tweaking commit below. Thank you, Junio.

~Sam

> 
> t/t1500-rev-parse.sh | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index b774cafc5d..01abee533d 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -134,6 +134,7 @@ test_expect_success 'rev-parse --is-shallow-repository in non-shallow repo' '
> test_expect_success 'showing the superproject correctly' '
> 	git rev-parse --show-superproject-working-tree >out &&
> 	test_must_be_empty out &&
> +
> 	test_create_repo super &&
> 	test_commit -C super test_commit &&
> 	test_create_repo sub &&
> @@ -142,20 +143,20 @@ test_expect_success 'showing the superproject correctly' '
> 	echo $(pwd)/super >expect  &&
> 	git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> 	test_cmp expect out &&
> +
> 	test_commit -C super submodule_add &&
> 	git -C super checkout -b branch1 &&
> 	git -C super/dir/sub checkout -b branch1 &&
> 	test_commit -C super/dir/sub branch1_commit &&
> 	git -C super add dir/sub &&
> 	test_commit -C super branch1_commit &&
> -	git -C super checkout master &&
> -	git -C super checkout -b branch2 &&
> -	git -C super/dir/sub checkout master &&
> -	git -C super/dir/sub checkout -b branch2 &&
> +	git -C super checkout -b branch2 master &&
> +	git -C super/dir/sub checkout -b branch2 master &&
> 	test_commit -C super/dir/sub branch2_commit &&
> 	git -C super add dir/sub &&
> 	test_commit -C super branch2_commit &&
> 	test_must_fail git -C super merge branch1 &&
> +
> 	git -C super/dir/sub rev-parse --show-superproject-working-tree >out &&
> 	test_cmp expect out
> '