[PATCH i-g-t] CONTRIBUTING: Add more guidelines

[PATCH i-g-t] CONTRIBUTING: Add more guidelines

[Kamil Konieczny]

Add few more guidelines for submitting patches:
- encourage use of checkpatch.pl script from Linux kernel
- add guide for good subject for a patch
- add links to guides about how to create a good patch
- add guide for responding to CI failure letters

v1: expand patch from Louis on checkpatch,
  also take a hint from Jani about checkpatch options
  add a guide for checking test results from CI

Cc: Arthur Grillo <arthurgrillo@riseup.net>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Petri Latvala <adrinael@adrinael.net>
Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 CONTRIBUTING.md | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 2ab8a48f6..23cf0df11 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -63,6 +63,17 @@ Sending Patches
 
   on its first invocation.
 
+- Place relevant prefix in subject, for example when your change is in one
+  testfile, use its name without '.c' nor '.h' suffix, like:
+  tests/simple_test: short description
+  Consider sending cover letter with your patch, so if you decide to change
+  subject it can still be linked into same patchseries on patchwork.
+
+- Look into some guides from Linux and Open Source community:
+  https://kernelnewbies.org/PatchPhilosophy
+  https://www.kernel.org/doc/html/latest/process/submitting-patches.html
+  https://www.kernel.org/doc/html/latest/process/submit-checklist.html
+
 - Patches need to be reviewed on the mailing list. Exceptions only apply for
   testcases and tooling for drivers with just a single contributor (e.g. vc4).
   In this case patches must still be submitted to the mailing list first.
@@ -75,8 +86,17 @@ Sending Patches
   contact one of the maintainers (listed in the MAINTAINERS file) and cc the
   igt-dev mailing list.
 
+- Before sending use Linux kernel script 'checkpatch.pl' for checking your
+  patchset. You could ignore some of them like 'line too long' or 'typdef'
+  but most of the time its log is accurate. Useful options you could use:
+  --emacs --strict --show-types --max-line-length=100 \
+  --ignore=BIT_MACRO,SPLIT_STRING,LONG_LINE_STRING,BOOL_MEMBER
+
 - Changes to the testcases are automatically tested. Take the results into
-  account before merging.
+  account before merging.  Please also reply to CI failures if you think they
+  are unrelated, add also to Cc CI e-mail which is present in message.  This
+  can help our bug-filing team. When replying, you can cut a message after
+  'Known bugs' to keep it in reasonable size.
 
 
 Commit Rights
-- 
2.43.0

Re: [PATCH i-g-t] CONTRIBUTING: Add more guidelines

[Louis Chauvet]

Thanks for this update, it is better than my initial wording.

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

Thanks,
Louis Chauvet

Le 29/05/24 - 14:48, Kamil Konieczny a écrit :
> Add few more guidelines for submitting patches:
> - encourage use of checkpatch.pl script from Linux kernel
> - add guide for good subject for a patch
> - add links to guides about how to create a good patch
> - add guide for responding to CI failure letters
> 
> v1: expand patch from Louis on checkpatch,
>   also take a hint from Jani about checkpatch options
>   add a guide for checking test results from CI
> 
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> Cc: Louis Chauvet <louis.chauvet@bootlin.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Petri Latvala <adrinael@adrinael.net>
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  CONTRIBUTING.md | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 2ab8a48f6..23cf0df11 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -63,6 +63,17 @@ Sending Patches
>  
>    on its first invocation.
>  
> +- Place relevant prefix in subject, for example when your change is in one
> +  testfile, use its name without '.c' nor '.h' suffix, like:
> +  tests/simple_test: short description
> +  Consider sending cover letter with your patch, so if you decide to change
> +  subject it can still be linked into same patchseries on patchwork.
> +
> +- Look into some guides from Linux and Open Source community:
> +  https://kernelnewbies.org/PatchPhilosophy
> +  https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> +  https://www.kernel.org/doc/html/latest/process/submit-checklist.html
> +
>  - Patches need to be reviewed on the mailing list. Exceptions only apply for
>    testcases and tooling for drivers with just a single contributor (e.g. vc4).
>    In this case patches must still be submitted to the mailing list first.
> @@ -75,8 +86,17 @@ Sending Patches
>    contact one of the maintainers (listed in the MAINTAINERS file) and cc the
>    igt-dev mailing list.
>  
> +- Before sending use Linux kernel script 'checkpatch.pl' for checking your
> +  patchset. You could ignore some of them like 'line too long' or 'typdef'
> +  but most of the time its log is accurate. Useful options you could use:
> +  --emacs --strict --show-types --max-line-length=100 \
> +  --ignore=BIT_MACRO,SPLIT_STRING,LONG_LINE_STRING,BOOL_MEMBER
> +
>  - Changes to the testcases are automatically tested. Take the results into
> -  account before merging.
> +  account before merging.  Please also reply to CI failures if you think they
> +  are unrelated, add also to Cc CI e-mail which is present in message.  This
> +  can help our bug-filing team. When replying, you can cut a message after
> +  'Known bugs' to keep it in reasonable size.
>  
>  
>  Commit Rights
> -- 
> 2.43.0
> 

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH i-g-t] CONTRIBUTING: Add more guidelines

[Zbigniew Kempczyński]

On Wed, May 29, 2024 at 02:48:49PM +0200, Kamil Konieczny wrote:
> Add few more guidelines for submitting patches:
> - encourage use of checkpatch.pl script from Linux kernel
> - add guide for good subject for a patch
> - add links to guides about how to create a good patch
> - add guide for responding to CI failure letters
> 
> v1: expand patch from Louis on checkpatch,
>   also take a hint from Jani about checkpatch options
>   add a guide for checking test results from CI
> 
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> Cc: Louis Chauvet <louis.chauvet@bootlin.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Petri Latvala <adrinael@adrinael.net>
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  CONTRIBUTING.md | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 2ab8a48f6..23cf0df11 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -63,6 +63,17 @@ Sending Patches
>  
>    on its first invocation.
>  
> +- Place relevant prefix in subject, for example when your change is in one
> +  testfile, use its name without '.c' nor '.h' suffix, like:
> +  tests/simple_test: short description
> +  Consider sending cover letter with your patch, so if you decide to change
> +  subject it can still be linked into same patchseries on patchwork.
> +
> +- Look into some guides from Linux and Open Source community:
> +  https://kernelnewbies.org/PatchPhilosophy
> +  https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> +  https://www.kernel.org/doc/html/latest/process/submit-checklist.html
> +
>  - Patches need to be reviewed on the mailing list. Exceptions only apply for
>    testcases and tooling for drivers with just a single contributor (e.g. vc4).
>    In this case patches must still be submitted to the mailing list first.
> @@ -75,8 +86,17 @@ Sending Patches
>    contact one of the maintainers (listed in the MAINTAINERS file) and cc the
>    igt-dev mailing list.
>  
> +- Before sending use Linux kernel script 'checkpatch.pl' for checking your
> +  patchset. You could ignore some of them like 'line too long' or 'typdef'
> +  but most of the time its log is accurate. Useful options you could use:
> +  --emacs --strict --show-types --max-line-length=100 \
> +  --ignore=BIT_MACRO,SPLIT_STRING,LONG_LINE_STRING,BOOL_MEMBER

My last series exercised by checkpatch.pl noticed one warning which I ignored,
but with the above there's another one I definitely should address.

> +
>  - Changes to the testcases are automatically tested. Take the results into
> -  account before merging.
> +  account before merging.  Please also reply to CI failures if you think they
> +  are unrelated, add also to Cc CI e-mail which is present in message.  This
> +  can help our bug-filing team. When replying, you can cut a message after
> +  'Known bugs' to keep it in reasonable size.
>  
>  
>  Commit Rights
> -- 
> 2.43.0
> 

Maybe we should either consider using checkpatch.pl in CI as additional
step. It would be useful to see this on patchwork as well.

Regardless comment above, this patch is:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

Re: [PATCH i-g-t] CONTRIBUTING: Add more guidelines

[Arthur Grillo]

On 29/05/24 09:48, Kamil Konieczny wrote:
> Add few more guidelines for submitting patches:
> - encourage use of checkpatch.pl script from Linux kernel
> - add guide for good subject for a patch
> - add links to guides about how to create a good patch
> - add guide for responding to CI failure letters

Good guidelines! I myself didn't know about the CI part :P.

Reviewed-by: Arthur Grillo <arthurgrillo@riseup.net>

Best Regards,
~Arthur Grillo

> 
> v1: expand patch from Louis on checkpatch,
>   also take a hint from Jani about checkpatch options
>   add a guide for checking test results from CI
> 
> Cc: Arthur Grillo <arthurgrillo@riseup.net>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> Cc: Louis Chauvet <louis.chauvet@bootlin.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Petri Latvala <adrinael@adrinael.net>
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  CONTRIBUTING.md | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 2ab8a48f6..23cf0df11 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -63,6 +63,17 @@ Sending Patches
>  
>    on its first invocation.
>  
> +- Place relevant prefix in subject, for example when your change is in one
> +  testfile, use its name without '.c' nor '.h' suffix, like:
> +  tests/simple_test: short description
> +  Consider sending cover letter with your patch, so if you decide to change
> +  subject it can still be linked into same patchseries on patchwork.
> +
> +- Look into some guides from Linux and Open Source community:
> +  https://kernelnewbies.org/PatchPhilosophy
> +  https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> +  https://www.kernel.org/doc/html/latest/process/submit-checklist.html
> +
>  - Patches need to be reviewed on the mailing list. Exceptions only apply for
>    testcases and tooling for drivers with just a single contributor (e.g. vc4).
>    In this case patches must still be submitted to the mailing list first.
> @@ -75,8 +86,17 @@ Sending Patches
>    contact one of the maintainers (listed in the MAINTAINERS file) and cc the
>    igt-dev mailing list.
>  
> +- Before sending use Linux kernel script 'checkpatch.pl' for checking your
> +  patchset. You could ignore some of them like 'line too long' or 'typdef'
> +  but most of the time its log is accurate. Useful options you could use:
> +  --emacs --strict --show-types --max-line-length=100 \
> +  --ignore=BIT_MACRO,SPLIT_STRING,LONG_LINE_STRING,BOOL_MEMBER
> +
>  - Changes to the testcases are automatically tested. Take the results into
> -  account before merging.
> +  account before merging.  Please also reply to CI failures if you think they
> +  are unrelated, add also to Cc CI e-mail which is present in message.  This
> +  can help our bug-filing team. When replying, you can cut a message after
> +  'Known bugs' to keep it in reasonable size.
>  
>  
>  Commit Rights