Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> 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)
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
Attachment:
signature.asc
Description: PGP signature
