Re: [PATCH v2] t7001-mv.sh: modernizing test script using functions

From: Martin Ågren <martin.agren@gmail.com>
Date: Nov 3, 2022, 19:37 UTC
Message-ID: <CAN0heSohT_zDYequyv9-cCALrbpR0yQnSoPA3+YGMhgKxE5K3g@mail.gmail.com>
In-Reply-To: <pull.1372.v2.git.git.1667500788132.gitgitgadget@gmail.com>

On Thu, 3 Nov 2022 at 19:39, Debra Obondo via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 12 quoted lines

>
> From: Debra Obondo <debraobondo@gmail.com>
>
> Test script to verify the presence/absence of files, paths, directories,
> symlinks and other features in 'git mv' command are using the command
> format:
>
> 'test (-e|f|d|h|...)'
>
> Replace them with helper functions of format:
>
> 'test_path_is_*'

Ok.

Show 10 quoted lines

> Changes since v1
>
> Replacing idiomatic helper functions
>
> '! test_path_is_*'
>
> with
>
> 'test_path_is_missing'
>

The above "Changes since v1" and what I've quoted here should probably be dropped. We prefer our patches to look as if they've appeared out of the blue in perfect shape. :-)

> This uses values of 'test_path_bar' in place of '! test_path_foo' to
> bring in the helpful factor of indicating the failure of tests after the
> mv command has been used, that is, it echoes if the feature/test_path
> exists.

This paragraph is excellent. It describes why you've done the patch this way. This looks much better than version 1 of the patch!

> Signed-off-by: Debra Obondo <debraobondo@gmail.com>

After removing the lines about changes since v1, this patch is

Reviewed-by: Martin Ågren <martin.agren@gmail.com>

Show 13 quoted lines

> ---
>     [PATCH v2] [OUTREACHY] t7001-mv.sh : Use test_path_is_* functions in
>     test script
>
>     Changes since v1:
>
>     Replacing idiomatic helper functions
>
>     '! test_path_is_*'
>
>     with
>
>     'test_path_is_missing'

This place after the "---" line is an excellent place for including such "here's what has changed since v1" comments. Good. They will not appear in the final commit message.

Thanks for contributing!

Martin