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

From: Lyude Paul <lyude@redhat.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH i-g-t] CONTRIBUTING: formalize review rules
Date: Tue, 18 Jul 2017 12:31:41 -0400	[thread overview]
Message-ID: <1500395501.9822.0.camel@redhat.com> (raw)
In-Reply-To: <20170718160020.6393-1-daniel.vetter@ffwll.ch>

Acked-by: Lyude <lyude@redhat.com>

On Tue, 2017-07-18 at 18:00 +0200, 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@collabora.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>
> Acked-by: Imre Deak <imre.deak@intel.com>
> Acked-by: Robert Foss <robert.foss@collabora.com>
> Acked-by: Ben Widawsky <ben@bwidawsk.net>
> Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Acked-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx