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

[Lionel Landwerlin]

Acked-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

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 at collabora.com>
> Acked-by: Jani Nikula <jani.nikula at intel.com>
> Acked-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Acked-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Acked-by: Petri Latvala <petri.latvala at intel.com>
> Acked-by: Imre Deak <imre.deak at intel.com>
> Acked-by: Robert Foss <robert.foss at collabora.com>
> Acked-by: Ben Widawsky <ben at bwidawsk.net>
> Acked-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Acked-by: Mika Kuoppala <mika.kuoppala at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>   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