Message ID | 20231016195549.2243243-1-tgamblin@baylibre.com |
---|---|
State | New |
Headers | show |
Series | contributor-guide: add patchtest section | expand |
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.
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 --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 -------------------------
Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com> --- .../contributor-guide/submit-changes.rst | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+)