patchModernize Test Path Checking: test -(e|f|d)

>  t/chainlint/chained-subshell.expect | 2 +-
>  t/chainlint/chained-subshell.test   | 2 +-
>  t/chainlint/function.expect         | 2 +-
>  t/chainlint/function.test           | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

> Thanks for submitting this GSoC microproject. See comments below...
>
> On Tue, Mar 18, 2025 at 7:58 AM Sampriyo Guin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> From: rimo <sampriyoguin@gmail.com>
>
> This name should match the Signed-off-by: name. Since the "From:"
> header is generated from the author information in the commit, you
> probably need to adjust your "user.name" configuration to fix this.
>
>> test -e changed to test_path_exists
>> test -f changed to test_path_is_file
>
> People reading the patch would like to know why a change is being
> made, so this is where you should explain the reason (for instance,
> "the test_path_* functions provide better diagnostics upon failure" or
> such). As Karthik mentioned[*], read the "Describe your changes well"
> section in Documentation/SubmittingPatches to learn how to craft a
> good commit message.
>
> [*]: https://lore.kernel.org/git/CAOLa=ZSkMp+H9PZeBZXK47=fx1sH=S54AuPT=oUosm7F7V8MGg@mail.gmail.com/
>
>> Signed-off-by: Sampriyo Guin <sampriyoguin@gmail.com>
>> ---
>>     , Jialuo She shejialuo@gmail.com , Christian Couder
>>     christian.couder@gmail.com, Ghanshyam Thakkar shyamthakkar001@gmail.com
>
> It appears that GitGitGadget didn't like how this list was formatted.
> Instead, place each recipient on its own Cc: line.
>
>>  t/chainlint/chained-subshell.expect | 2 +-
>>  t/chainlint/chained-subshell.test   | 2 +-
>>  t/chainlint/function.expect         | 2 +-
>>  t/chainlint/function.test           | 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> Let's not touch any of the "chainlint" files; they are checking
> validity of a completely separate tool ("chainlint"), and have nothing
> to do with checking Git itself. Instead, pick one of the t/t???-*.sh
> files.

> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Tue, Mar 18, 2025 at 7:58 AM Sampriyo Guin via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >>  t/chainlint/chained-subshell.expect | 2 +-
> >>  t/chainlint/chained-subshell.test   | 2 +-
> >
> > Let's not touch any of the "chainlint" files; they are checking
> > validity of a completely separate tool ("chainlint"), and have nothing
> > to do with checking Git itself. Instead, pick one of the t/t???-*.sh
> > files.
>
> Yeah, these changes to make them use test_path_* are not "fixes" but
> something else.  The first step for a contributor is to understand
> why "test_path_*" are preferred over "test -[def]" and in what
> context, but touching these files shows that such understanding is
> missing, unfortunately.
>
> I find the "as specified in Git Microprojects" in the patch
> description the most disturbing,
>
>     A simple fix as specified in Git Microprojects.
>
> as it may be an indication that some introductory write-up is
> misleading potential students in a wrong direction.  Our mentors may
> need a bit more handholding at this early stage of dipping your toes
> in the water step, perhaps?  Or is it up to the aspiring students to
> do their homework?

> On Tue, Mar 18, 2025 at 3:34 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > On Tue, Mar 18, 2025 at 7:58 AM Sampriyo Guin via GitGitGadget
>> > <gitgitgadget@gmail.com> wrote:
>> >>  t/chainlint/chained-subshell.expect | 2 +-
>> >>  t/chainlint/chained-subshell.test   | 2 +-
>> >
>> > Let's not touch any of the "chainlint" files; they are checking
>> > validity of a completely separate tool ("chainlint"), and have nothing
>> > to do with checking Git itself. Instead, pick one of the t/t???-*.sh
>> > files.
>>
>> Yeah, these changes to make them use test_path_* are not "fixes" but
>> something else.  The first step for a contributor is to understand
>> why "test_path_*" are preferred over "test -[def]" and in what
>> context, but touching these files shows that such understanding is
>> missing, unfortunately.
>>
>> I find the "as specified in Git Microprojects" in the patch
>> description the most disturbing,
>>
>>     A simple fix as specified in Git Microprojects.
>>
>> as it may be an indication that some introductory write-up is
>> misleading potential students in a wrong direction.  Our mentors may
>> need a bit more handholding at this early stage of dipping your toes
>> in the water step, perhaps?  Or is it up to the aspiring students to
>> do their homework?
>
> I'm not sure where the GSoC microproject ideas are maintained these
> days, but it may indeed be the case that (at least this microproject)
> could be spelled out in more detail to help lead newcomers in the
> correct direction. If not already mentioned, at least these
> clarifications probably ought to be made:
>
> * only work on t/t????-*.sh scripts
>
> * pick just one script (so as to avoid exhausting the pool for other candidates)
>
> * only convert `test -[def]` instances which semantically are
> assertions (i.e. used as part of a &&-chain)