Open Stack

From: Eric LEMOINE <elemoine at mirantis.com<mailto:elemoine at mirantis.com>>
Reply-To: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org<mailto:openstack-dev at lists.openstack.org>>
Date: Monday, February 22, 2016 at 1:13 AM
To: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org<mailto:openstack-dev at lists.openstack.org>>
Subject: Re: [openstack-dev] [kolla] discussion about core reviewer limitations by company


Le 20 févr. 2016 18:12, "Steven Dake (stdake)" <stdake at cisco.com<mailto:stdake at cisco.com>> a écrit :
>
> Hey folks,
>
> Mirantis has been developing a big footprint in the core review team, and Red Hat already has a big footprint in the core review team.  These are all good things, but I want to avoid in the future a situation in which one company has a majority of core reviewers.  Since core reviewers set policy for the project, the project could be harmed if one company has such a majority.  This is one reason why project diversity is so important and has its own special snowflake tag in the governance repository.
>
> I'd like your thoughts on how to best handle this situation, before I trigger  a vote we can all agree on.
>
> I was thinking of something simple like:
> "1 company may not have more then 33% of core reviewers.  At the conclusion of PTL elections, the current cycle's 6 months of reviews completed will be used as a metric to select the core reviewers from that particular company if the core review team has shrunk as a result of removal of core reviewers during the cycle."
>
> Thoughts, comments, questions, concerns, etc?


I think that introducing this policy would not be fair and would even be counter-productive. For example, my motivation would be low if I knew I couldn't be accepted as a core reviewer because my company already has "too many" core reviewers.  So this policy would close the doors to developers who could potentially be great contributors to the success of the project.  If anything, a policy where "2 people from the same company should not approve a third person from that same company's patch" (as stated by Sam Yaple) would sound more acceptable to me.


Eric,

Life isn't fair :(  That said, thank you for your feedback.  We definitely don't want to demotivate people as we balance the core reviewer team over time.

The issue isn't about reviewing patches in my opinion.  Obviously people shouldn't jam patches through the review queue that they know will be counter-productive to the majority view of the core reviewers.  If they do, they can easily be reverted by 3 different core reviewers.  Our core reviewers are adults and don't behave in this way.  I have seen a couple patches "jammed through" and not reverted, from multiple companies rather then just one company and it just made everyone angry.  I think folks have learned from that and tend not to "jan through contentious changes" unless it is time critical (as in breaking gate, or busted master, or milestone deadline).

The issue is around policy setting.  The PTL should NOT be a dictator and set policy on their own whim.  The way we set policy in Kolla (and I believe in other OpenStack projects) is by formal majority vote.  For example, one policy we have set is that we permit third party proprietary distros and plugins to interact and even be a part of our Dockerfile.j2 if someone steps up to maintain them.  NB our specs directory are actually policy "direction" rather then hard policies.  That is why spec's require a majority vote to approve.

Folks that have responded on this thread thus far have seem to missed this policy point and focused on the reviewing of patches.

All that said, I hear what your saying regarding motivation.  The original discussion was about protecting the project from a lack of diversity in the core reviewer team which could potentially lead to majority-rules by one corporate affiliation on policy matters.  What would be an ideal outcome in my opinion is to keep motivation intact but meet the diversity requirements set forth in the governance repository to avoid a majority-rules by one corporate affiliation situation.  There are two ways to do this that I can think of:

Add core reviewers that aren't quite there yet, but close to meet the diversity requirements
Or
Limit core reviewers
Or
Another simple solution is to permit a a veto vote from any core reviewer within the 1 week voting window if a majority (or some other value, such as 35%) from one corporate affiliation votes yes on a policy decision.  This could be gamed, but at least does not permit policy changes by one corporate affiliation.  With our current core review team, that means 3 people could vote from RHT (out of the 4 core reviewers) before triggering the veto rule.
Or
Permit a veto vote on policy changes (I really don't like this option, as it gives too much "power" to one individual over the project policy)

I'd like to hear what other core reviewers as well as Kolla developers have to say about the matter.

As a final note, I am very very (!) anti-process.  A project should only have as much process as it needs to succeed.  Many/most projects(not OpenStack, but other projects) go overboard on process.  Process just creates needless complication, so I am also not hot on setting a bunch of policies (which require process to execute).  The main problem with process is it creates too many rules for people to make sure they are compliant with.  This slows the system down, and the system should be fast, nimble, and agile.

When I open discussion on a policy change, its not like I do it for my health - its because I see a serious issue coming down the road.  In this case I don't know precisely how to correct this particular problem, which is why we are having this discussion.  I'd prefer if folks focus on what we can do to fix it, rather then saying "no lets not do anything".

Regards
-steve


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20160222/7c3f85dd/attachment.html>