diff mbox series

docs: submitting-patches: Introduce Test: tag

Message ID 20231007-topic-test_tag-v1-1-513cd9e577ed@linaro.org
State New
Headers
Series docs: submitting-patches: Introduce Test: tag |

Commit Message

Konrad Dybcio Oct. 7, 2023, 12:43 a.m. UTC 
  Currently, we blindly trust the submitters that they both compiled their
code at all, tested it on a relevant device, and have done so in a manner
that made sense for a given changeset.

If at least two of these three things were always true, the review
workflow would be much more exciting.

Introduce a new Test: tag to help submitters express the way the patch
was tested, making it easier to understand for reviewers and maintainers
whether it was tested, and if so, whether that test was sufficient.

I originally found something like this on Google's Android kernel repos
and loved the concept.

Test: make htmldocs and manual examination
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 Documentation/process/submitting-patches.rst | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)


---
base-commit: 0f0fe5040de5e5fd9b040672e37725b046e312f0
change-id: 20231007-topic-test_tag-72bca62617f9

Best regards,

Comments

Jonathan Corbet Oct. 7, 2023, 12:57 p.m. UTC | #1 
Konrad Dybcio <konrad.dybcio@linaro.org> writes:

> Currently, we blindly trust the submitters that they both compiled their
> code at all, tested it on a relevant device, and have done so in a manner
> that made sense for a given changeset.
>
> If at least two of these three things were always true, the review
> workflow would be much more exciting.
>
> Introduce a new Test: tag to help submitters express the way the patch
> was tested, making it easier to understand for reviewers and maintainers
> whether it was tested, and if so, whether that test was sufficient.
>
> I originally found something like this on Google's Android kernel repos
> and loved the concept.
>
> Test: make htmldocs and manual examination
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  Documentation/process/submitting-patches.rst | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Do we really want to do this?  To me, it almost seems like it codifies
the idea that sending *untested* patches is OK as long as you leave out
the tag.

Others may disagree, but I don't think we need yet another tag for this.
Testing of patches before sending them should be the norm; if special
notes about testing are needed, they can go in or below the changelog,
as appropriate.

Thanks,

jon
Geert Uytterhoeven Oct. 8, 2023, 5:18 p.m. UTC | #2 
On Sat, Oct 7, 2023 at 2:57 PM Jonathan Corbet <corbet@lwn.net> wrote:
> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>
> > Currently, we blindly trust the submitters that they both compiled their
> > code at all, tested it on a relevant device, and have done so in a manner
> > that made sense for a given changeset.
> >
> > If at least two of these three things were always true, the review
> > workflow would be much more exciting.
> >
> > Introduce a new Test: tag to help submitters express the way the patch
> > was tested, making it easier to understand for reviewers and maintainers
> > whether it was tested, and if so, whether that test was sufficient.
> >
> > I originally found something like this on Google's Android kernel repos
> > and loved the concept.
> >
> > Test: make htmldocs and manual examination
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > ---
> >  Documentation/process/submitting-patches.rst | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
>
> Do we really want to do this?  To me, it almost seems like it codifies
> the idea that sending *untested* patches is OK as long as you leave out
> the tag.

Exactly. We are already receiving too many untested patches.

> Others may disagree, but I don't think we need yet another tag for this.
> Testing of patches before sending them should be the norm; if special

+1

> notes about testing are needed, they can go in or below the changelog,
> as appropriate.

+1

Gr{oetje,eeting}s,

                        Geert
Konrad Dybcio Oct. 9, 2023, 10:14 a.m. UTC | #3 
On 8.10.2023 19:18, Geert Uytterhoeven wrote:
> On Sat, Oct 7, 2023 at 2:57 PM Jonathan Corbet <corbet@lwn.net> wrote:
>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>
>>> Currently, we blindly trust the submitters that they both compiled their
>>> code at all, tested it on a relevant device, and have done so in a manner
>>> that made sense for a given changeset.
>>>
>>> If at least two of these three things were always true, the review
>>> workflow would be much more exciting.
>>>
>>> Introduce a new Test: tag to help submitters express the way the patch
>>> was tested, making it easier to understand for reviewers and maintainers
>>> whether it was tested, and if so, whether that test was sufficient.
>>>
>>> I originally found something like this on Google's Android kernel repos
>>> and loved the concept.
>>>
>>> Test: make htmldocs and manual examination
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>>  Documentation/process/submitting-patches.rst | 18 +++++++++++++++++-
>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> Do we really want to do this?  To me, it almost seems like it codifies
>> the idea that sending *untested* patches is OK as long as you leave out
>> the tag.
> 
> Exactly. We are already receiving too many untested patches.
> 
>> Others may disagree, but I don't think we need yet another tag for this.
>> Testing of patches before sending them should be the norm; if special
> 
> +1
> 
>> notes about testing are needed, they can go in or below the changelog,
>> as appropriate.
> 
> +1
> 
Okay, I see your points, let's forget about this..

Konrad
diff mbox series

Patch

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 3fcfa029c9b3..c3fda5743ca7 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -522,7 +522,7 @@  Example of a patch submitted by a Co-developed-by: author::
 	Signed-off-by: Submitting Co-Author <sub@coauthor.example.org>
 
 
-Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
+Using informative tags:
 ----------------------------------------------------------------------
 
 The Reported-by tag gives credit to people who find bugs and report them and it
@@ -600,6 +600,22 @@  process nor the requirement to Cc: stable@vger.kernel.org on all stable
 patch candidates. For more information, please read
 Documentation/process/stable-kernel-rules.rst.
 
+A Test: tag confirms that the patch was actually tested by the submitter and
+helps reviewers determine whether the testing procedure made sense for a given
+changeset. The latter in particular, may bring attention to errors in the
+testing procedure and prompt a more in-depth examination of a patch.
+
+Commonly, ``Test: Smoke test on [device name]`` may be used to signify that:
+
+	 (a) The kernel compiled successfully with the default defconfig.
+
+	 (b) The device has successfully booted the image from point (a), with
+	     no apparent loss in functionality compared to the state before this
+	     patch was applied.
+
+	 (c) The submitter believes in good faith, that such simple test is
+	     enough, given the scope of the patch.
+
 .. _the_canonical_patch_format:
 
 The canonical patch format