On Tue, Mar 18, 2025 at 3:34 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> > On Tue, Mar 18, 2025 at 7:58 AM Sampriyo Guin via GitGitGadget
> > <gitgitgadget@xxxxxxxxx> 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)
