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

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

[Daniel Vetter]

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
-- 
2.13.2

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

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

[Lyude Paul]

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

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

[Lionel Landwerlin]

Acked-by: Lionel Landwerlin <lionel.g.landwerlin@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@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

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

[Daniel Vetter]

On Tue, Jul 18, 2017 at 10:34 PM, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@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?

Just going out and getting the first r-b tag from the lowest bidder
would indeed be a bit silly, but I don't see any issue with new
contributors (who might not yet have pushed anything) doing review.

Imo review is about a lot more than just code correctess, it's also
really great tool for aligning a team on the ideas in a code, for
mentoring and learning and all that stuff. So someone new reviewing
code of someone experienced, and making the code and docs better by
asking questions, sounds pretty perfect to me.

Ofc two completely new contributors reviewing each another's stuff
would again defeat that, but then they need at least someone slightly
more experienced as committer again.

tldr; totally not worried nor seeing a need for criteria for reviewers.

Cheers, Daniel

> -
> 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@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
>
>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Tue, 18 Jul 2017, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Tue, Jul 18, 2017 at 10:34 PM, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
>> Acked-by: Lionel Landwerlin <lionel.g.landwerlin@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?
>
> Just going out and getting the first r-b tag from the lowest bidder
> would indeed be a bit silly, but I don't see any issue with new
> contributors (who might not yet have pushed anything) doing review.
>
> Imo review is about a lot more than just code correctess, it's also
> really great tool for aligning a team on the ideas in a code, for
> mentoring and learning and all that stuff. So someone new reviewing
> code of someone experienced, and making the code and docs better by
> asking questions, sounds pretty perfect to me.
>
> Ofc two completely new contributors reviewing each another's stuff
> would again defeat that, but then they need at least someone slightly
> more experienced as committer again.

To amend that, we put trust in people who we give commit access to, and
they need to assess whether the review has been sufficient. Rules to
cover all cases would be so long nobody would read them. This change in
review rules should steer the committers towards requiring more formal
reviewed-by on the list before pushing than has so far been the case,
and we expect the committers to follow.

BR,
Jani.

>
> tldr; totally not worried nor seeing a need for criteria for reviewers.
>
> Cheers, Daniel
>
>> -
>> 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@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
>>
>>
>>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2791 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> 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.

Thanks for considering my case here :)

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

[Arkadiusz Hiler]

On Tue, Jul 18, 2017 at 06:00:20PM +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>
Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

With the recent growth in contributions it's a good thing :-)

Thanks!

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

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

[Daniel Vetter]

On Wed, Jul 19, 2017 at 11:03:46AM +0300, Arkadiusz Hiler wrote:
> On Tue, Jul 18, 2017 at 06:00:20PM +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>
> Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> 
> With the recent growth in contributions it's a good thing :-)

And pushed, thx everyone for their acks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx