diff mbox series

contributor-guide: add patchtest section

Message ID 20231016195549.2243243-1-tgamblin@baylibre.com
State New
Headers show
Series contributor-guide: add patchtest section | expand

Commit Message

Trevor Gamblin Oct. 16, 2023, 7:55 p.m. UTC
Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
---
 .../contributor-guide/submit-changes.rst      | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Michael Opdenacker Oct. 17, 2023, 9:25 a.m. UTC | #1
Hi Trevor,

Many thanks for the patch!

I have a few quick tweaks to suggest, see below...

On 16.10.23 at 21:55, Trevor Gamblin wrote:
> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
> ---
>   .../contributor-guide/submit-changes.rst      | 37 +++++++++++++++++++
>   1 file changed, 37 insertions(+)
>
> diff --git a/documentation/contributor-guide/submit-changes.rst b/documentation/contributor-guide/submit-changes.rst
> index cda2d12d2..6dfb0ffec 100644
> --- a/documentation/contributor-guide/submit-changes.rst
> +++ b/documentation/contributor-guide/submit-changes.rst
> @@ -401,6 +401,43 @@ Anyway, you'll also be able to access the new messages on mailing list archives,
>   either through a web browser, or for the lists archived on https://lore.kernelorg,
>   through an individual newsgroup feed or a git repository.
>   
> +Validating Patches with Patchtest
> +=================================


Hey, what about having this section right after the "Creating Patches" 
and before "Sending the Patches via Email"? This would make more sense IMHO.

> +
> +Patchtest is available in ``openembedded-core`` as a tool for making sure
> +that your patches are well-formatted and contain important info for
> +maintenance purposes, such as ``Signed-off-by`` and ``Upstream-Status``
> +tags. Once you have generated the patch files and run ``source
> +oe-init-build-env`` to initialize your workspace, you can run patchtest
> +like so::


Here and in the rest of the patch, could you use "``patchtest``" instead 
of "Patchtest" or "patchtest"? This would be aligned with the way we 
refer to other tools such as devtool (see 
https://git.yoctoproject.org/yocto-docs/tree/documentation/standards.md#n74)

> +
> +    patchtest --patch <patch_name>
> +
> +Where ``/path/to/target/repo`` may (for example) be openembedded-core,
> +with the actual patchtest tests being kept in
> +``openembedded-core/meta/lib/patchtest/tests``.

Is this sentence relevant here, as "/path/to/target/repo" is not part of 
the above command line?

> Alternatively, if you
> +want patchtest to iterate over and test multiple patches stored in a
> +directory, you can use::
> +
> +    patchtest --directory <directory_name>
> +
> +By default, patchtest uses its own modules' file paths to determine what
> +repository and test suite to check patches against. If you wish to test
> +patches against a repository other than ``openembedded-core`` and/or use
> +a different set of tests, you can use the --repodir and --testdir


Use ``--repodir`` and ``--testdir``.

> +flags::
> +
> +    patchtest --patch <patch_name> --repodir <path/to/repo> --testdir <path/to/testdir>
> +
> +Finally, note that patchtest is designed to test patches in a standalone
> +way, so if your patches are meant to apply on top of changes made by
> +previous patches in a series, it is possible that patchtest will report
> +false failures regarding the "merge on head" test.
> +
> +Using patchtest in this manner provides a final check for the overall
> +quality of your changes before they are submitted for review by the
> +maintainers.


Very cool, I'm excited to use this tool, including for documentation 
patches ;-)

Of course, I'll apply your patch when patchtest makes it to oe-core.

Cheers
Michael.
Trevor Gamblin Oct. 17, 2023, 1:26 p.m. UTC | #2
On 2023-10-17 05:25, Michael Opdenacker wrote:
> Hi Trevor,
>
> Many thanks for the patch!
>
> I have a few quick tweaks to suggest, see below...

Hi Michael,

I'll send some revisions shortly.

- Trevor

>
> On 16.10.23 at 21:55, Trevor Gamblin wrote:
>> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
>> ---
>>   .../contributor-guide/submit-changes.rst      | 37 +++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/documentation/contributor-guide/submit-changes.rst 
>> b/documentation/contributor-guide/submit-changes.rst
>> index cda2d12d2..6dfb0ffec 100644
>> --- a/documentation/contributor-guide/submit-changes.rst
>> +++ b/documentation/contributor-guide/submit-changes.rst
>> @@ -401,6 +401,43 @@ Anyway, you'll also be able to access the new 
>> messages on mailing list archives,
>>   either through a web browser, or for the lists archived on 
>> https://lore.kernelorg,
>>   through an individual newsgroup feed or a git repository.
>>   +Validating Patches with Patchtest
>> +=================================
>
>
> Hey, what about having this section right after the "Creating Patches" 
> and before "Sending the Patches via Email"? This would make more sense 
> IMHO.
>
>> +
>> +Patchtest is available in ``openembedded-core`` as a tool for making 
>> sure
>> +that your patches are well-formatted and contain important info for
>> +maintenance purposes, such as ``Signed-off-by`` and ``Upstream-Status``
>> +tags. Once you have generated the patch files and run ``source
>> +oe-init-build-env`` to initialize your workspace, you can run patchtest
>> +like so::
>
>
> Here and in the rest of the patch, could you use "``patchtest``" 
> instead of "Patchtest" or "patchtest"? This would be aligned with the 
> way we refer to other tools such as devtool (see 
> https://git.yoctoproject.org/yocto-docs/tree/documentation/standards.md#n74)
>
>> +
>> +    patchtest --patch <patch_name>
>> +
>> +Where ``/path/to/target/repo`` may (for example) be openembedded-core,
>> +with the actual patchtest tests being kept in
>> +``openembedded-core/meta/lib/patchtest/tests``.
>
> Is this sentence relevant here, as "/path/to/target/repo" is not part 
> of the above command line?
>
>> Alternatively, if you
>> +want patchtest to iterate over and test multiple patches stored in a
>> +directory, you can use::
>> +
>> +    patchtest --directory <directory_name>
>> +
>> +By default, patchtest uses its own modules' file paths to determine 
>> what
>> +repository and test suite to check patches against. If you wish to test
>> +patches against a repository other than ``openembedded-core`` and/or 
>> use
>> +a different set of tests, you can use the --repodir and --testdir
>
>
> Use ``--repodir`` and ``--testdir``.
>
>> +flags::
>> +
>> +    patchtest --patch <patch_name> --repodir <path/to/repo> 
>> --testdir <path/to/testdir>
>> +
>> +Finally, note that patchtest is designed to test patches in a 
>> standalone
>> +way, so if your patches are meant to apply on top of changes made by
>> +previous patches in a series, it is possible that patchtest will report
>> +false failures regarding the "merge on head" test.
>> +
>> +Using patchtest in this manner provides a final check for the overall
>> +quality of your changes before they are submitted for review by the
>> +maintainers.
>
>
> Very cool, I'm excited to use this tool, including for documentation 
> patches ;-)
>
> Of course, I'll apply your patch when patchtest makes it to oe-core.
>
> Cheers
> Michael.
>
diff mbox series

Patch

diff --git a/documentation/contributor-guide/submit-changes.rst b/documentation/contributor-guide/submit-changes.rst
index cda2d12d2..6dfb0ffec 100644
--- a/documentation/contributor-guide/submit-changes.rst
+++ b/documentation/contributor-guide/submit-changes.rst
@@ -401,6 +401,43 @@  Anyway, you'll also be able to access the new messages on mailing list archives,
 either through a web browser, or for the lists archived on https://lore.kernelorg,
 through an individual newsgroup feed or a git repository.
 
+Validating Patches with Patchtest
+=================================
+
+Patchtest is available in ``openembedded-core`` as a tool for making sure
+that your patches are well-formatted and contain important info for
+maintenance purposes, such as ``Signed-off-by`` and ``Upstream-Status``
+tags. Once you have generated the patch files and run ``source
+oe-init-build-env`` to initialize your workspace, you can run patchtest
+like so::
+
+    patchtest --patch <patch_name>
+
+Where ``/path/to/target/repo`` may (for example) be openembedded-core,
+with the actual patchtest tests being kept in
+``openembedded-core/meta/lib/patchtest/tests``. Alternatively, if you
+want patchtest to iterate over and test multiple patches stored in a
+directory, you can use::
+
+    patchtest --directory <directory_name>
+
+By default, patchtest uses its own modules' file paths to determine what
+repository and test suite to check patches against. If you wish to test
+patches against a repository other than ``openembedded-core`` and/or use
+a different set of tests, you can use the --repodir and --testdir
+flags::
+
+    patchtest --patch <patch_name> --repodir <path/to/repo> --testdir <path/to/testdir>
+
+Finally, note that patchtest is designed to test patches in a standalone
+way, so if your patches are meant to apply on top of changes made by
+previous patches in a series, it is possible that patchtest will report
+false failures regarding the "merge on head" test.
+
+Using patchtest in this manner provides a final check for the overall
+quality of your changes before they are submitted for review by the
+maintainers.
+
 Sending Patches via Email
 -------------------------