Eric Sunshine [off-list ref] writes:

On Tue, Mar 18, 2025 at 3:34 PM Junio C Hamano [off-list ref] wrote:

quoted

Eric Sunshine [off-list ref] writes:

quoted

On Tue, Mar 18, 2025 at 7:58 AM Sampriyo Guin via GitGitGadget
[off-list ref] wrote:

quoted

 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)

I think these are really good points, we ought to add to the
microproject page, I've raised a pull request on the repository to do so
[1].

[1]: https://github.com/git/git.github.io/pull/760