Re: [OUTREACHY][PATCH v2] t7006: Use test_path_is_* functions in test script

[Jonathan Nieder <jrnieder@xxxxxxxxx>]

Joey Salazar wrote:
> On Monday, October 26, 2020 5:00 PM, Joey Salazar <jgsal@xxxxxxxxxxxxxx> wrote:

>> Modernize the test by replacing `test -e` instances with
>> `test_path_is_file` helper functions, and `! test -e` with
>> `test_path_is_missing`, for better readability and diagnostic messages.
>> For instances when `${if_local_config}` is either '' or '! ' then
>> `test_path_is_file` will diagnose the directory and print a message if
>> and only if the result `is false` goes away.
>
> If `if test -n '$pager_wanted'` is checking if pager_wanted=true
> before diagnosing core.pager_used, then would;
>
> For other instances when '$pager_wanted' is not empty then `test_path_is_file`
> will diagnose the directory and print a message.
>
> be more accurate?

Yes, but because it restates what the patch says instead of describing
the "why", it's at the wrong level of abstraction.

Starting from

    t7006: Use test_path_is_* functions in test script

    Modernize the test by replacing `test -e` instances with
    `test_path_is_file` helper functions, and `! test -e` with
    `test_path_is_missing`, for better readability and diagnostic messages.

I think what would make sense is to add a second paragraph describing
why the existing code uses ${if_local_config} and why what the new
code is doing is better.

Thanks,
Jonathan