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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 3 Nov 2022 at 19:39, Debra Obondo via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Debra Obondo <debraobondo@xxxxxxxxx>
>
> 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.

> 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@xxxxxxxxx>

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

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

> ---
>     [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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux