Re: [PATCH i-g-t] CONTRIBUTING: formalize review rules

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Acked-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>

I assume review cannot be provided by someone who doesn't already contribute or has a number of patches in already.

What's the criteria to become a reviewer?
Is there is going to be a list of people to go to for review?

-
Lionel

On 18/07/17 17:00, Daniel Vetter wrote:
There's a bunch of reasons why I think we should formalize and enforce
our review rules for igt patches:

- We have a lot of new engineers joining and review helps enormously
   with mentoring and learning. But right now only patches from
   contributors without commit rights are consistently subjected to
   review, which makes this imbalanced and removes senior contributors
   from the review pool.

- We have a much bigger team and we need to make sure we're aligned on
   where igt as a tool and testsuite needs to head towards. Getting
   that alignment happens through reviewing each other's submission.
   Pushing a contentious patch and then dealing with a heated irc
   discussion is much less effective.

- Finally igt becomes ever more important for our testing, making sure
   the code quality is high is important. Review helps with that.

v2: Improve wording a bit (Imre).

Acked-by: Daniel Stone <daniels@xxxxxxxxxxxxx>
Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Acked-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Acked-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Acked-by: Petri Latvala <petri.latvala@xxxxxxxxx>
Acked-by: Imre Deak <imre.deak@xxxxxxxxx>
Acked-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>
Acked-by: Ben Widawsky <ben@xxxxxxxxxxxx>
Acked-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Acked-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
  CONTRIBUTING | 9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index d2adcf03b7c3..561c5dd80bba 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -26,10 +26,11 @@ A short list of contribution guidelines:
    convenience macros provided by the igt library. The semantic patch lib/igt.cocci
    can help with the more automatic conversions.
-- There is no formal review requirement and regular contributors with commit
-  access can push patches right after submitting them to the mailing lists. But
-  invasive changes, new helper libraries and contributions from newcomers should
-  go through a proper review to ensure overall consistency in the codebase.
+- 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.
+  Testcase should preferrably be cross-reviewed by the same people who write and
+  review the kernel feature itself.
- When patches from new contributors (without commit access) are stuck, for
    anything related to the regular releases, issues with packaging and


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux