Re: [PATCH v6 0/2] add-patch: classify '@' as a synonym for 'HEAD'

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Feb 14, 2024, 11:06 UTC
Message-ID: <497155c2-43d0-4e3f-884f-b0296a56823e@gmail.com>
In-Reply-To: <20240213000601.520731-2-shyamthakkar001@gmail.com>

Hi Ghanshyam

On 13/02/2024 00:05, Ghanshyam Thakkar wrote:

Show 6 quoted lines

> I just noticed after sending v5 that, in one of the tests, while
> moving it inside the loop for also testing '@', set_and_save_state was
> not used therefore the subsequent tests after the first $opt will not have
> the changes for which we prove 'y' thereby making the tests useless.
> (Although it would not fail regardless of this). This got unnoticed by
> the previous reviews since v1 and me as well.

This version all looks fine to me, thanks for working on it. Thanks for removing the PERL prerequisite from the remaining tests. I think the change to "git checkout @" is a good idea as it makes "@" act like "HEAD", hopefully there aren't too many users relying on "git checkout @" to detach HEAD.

Best Wishes

Phillip

Show 33 quoted lines

> Apologies for the oversight and noise.
> 
> Ghanshyam Thakkar (2):
>    add-patch: classify '@' as a synonym for 'HEAD'
>    add -p tests: remove PERL prerequisites
> 
>   add-patch.c                    |  8 ------
>   builtin/checkout.c             |  4 ++-
>   builtin/reset.c                |  4 ++-
>   t/t2016-checkout-patch.sh      | 46 +++++++++++++++++++---------------
>   t/t2020-checkout-detach.sh     | 12 +++++++++
>   t/t2024-checkout-dwim.sh       |  2 +-
>   t/t2071-restore-patch.sh       | 46 ++++++++++++++++++----------------
>   t/t3904-stash-patch.sh         |  6 -----
>   t/t7105-reset-patch.sh         | 38 +++++++++++++++-------------
>   t/t7106-reset-unborn-branch.sh |  2 +-
>   t/t7514-commit-patch.sh        |  6 -----
>   11 files changed, 92 insertions(+), 82 deletions(-)
> 
> change since v5:
> diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
> index 0f597416d8..453872c8ba 100755
> --- a/t/t7105-reset-patch.sh
> +++ b/t/t7105-reset-patch.sh
> @@ -29,6 +29,7 @@ test_expect_success PERL 'saying "n" does nothing' '
>   for opt in "HEAD" "@" ""
>   do
>          test_expect_success PERL "git reset -p $opt" '
> +               set_and_save_state dir/foo work work &&
>                  test_write_lines n y | git reset -p $opt >output &&
>                  verify_state dir/foo work head &&
>                  verify_saved_state bar &&
>