Re: [GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script

From: cheska fran <cheskaqiqi@gmail.com>
To: Andrei Rybak <rybak.a.v@gmail.com>, git@vger.kernel.org
Subject: Re: [GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script
Date: Thu, 2 Feb 2023 12:20:30 -0500	[thread overview]
Message-ID: <CAMO4yUHJCWGNV7esYqC7T4HB=pv3=Z4vTfaBFzh9hAsftfnN3w@mail.gmail.com> (raw)
In-Reply-To: <b9a38b83-cdfe-0389-3097-c20a699f183c@gmail.com>

Hi Andrei,

Andrei Rybak <rybak.a.v@gmail.com> On 31/01/2023 21:21：
> For patches that change a single test, the subject line can include just
> the "t" and the number.  The part after the colon should start with a
> lowercase letter.  Something like
>
>      t4113: modernize test style
>

Thanks, this tip is really helpful. I will change it.

> Commit message should start with description of the existing problem
> in present tense, something like:
>
>      Test scripts in file t4113-apply-ending.sh are written in old style,
>      where the test_expect_success command and test title are written on
>      separate lines ...
>
> Then changes should be described using imperative mood, as if you are
> giving commands to the codebase.  See section "[[describe-changes]]"
> in "Documentation/SubmittingPatches" for details.

> You can also find examples of existing commit messages for similar
> changes:
>
>      $ git log --no-merges --grep='modernize' -- t
>
Thanks,that is cool! I tried it and I saw a lot of examples and their
descriptions were very clear and I learned a lot

> In case of t4113, files "test-patch" and "file" are created twice.
> The second creation of the files could be either its own step
> 'setup for apply at the beginning', or incorporated into the step
> 'apply at the beginning'.
yeah,once before the first instance of test-patch and then again
before the second instance of test-patch.
I will move the second creation of the files to its own step in setup
for apply at the beginning.'

> Section "Recommended style" in t/README also has some notes about
> how heredocs should be indented.

Sure, I did not realize this.I will use "<<-" instead of "<<".

> While changing the quoting around test tiles and commands, the
> indentation with spaces could also be changed to TABs.
will do.

> If the setup code on top level of the file is moved into test
> steps, this comment and the "# setup" comment at line 11 will
> become unnecessary.
Thanks. It's easy to miss.The purpose of these tags is to distinguish
the setup and test parts of the script,
but if the file creation has been moved to a separate step, then these
tags are no longer needed.

Thanks for the reply and it is really helpful!

--
Thanks,
Shuqi