[openstack-dev] [all][infra][tc][ptl] Scaling up code review process (subdir cores)

Ihar Hrachyshka ihrachys at redhat.com
Wed Jun 3 12:09:56 UTC 2015


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 06/03/2015 01:56 PM, Boris Pavlovic wrote:
> Ihar,
> 
> Reverting patches is unacceptable for Rally project. This means
> that we merged bug and this is epic fail of PTL of project.
> 

That's a bar set too high. Though I don't believe Rally team does not
merge any buggy code, ever. Neither I believe in elves.

> 
> Let's take a look from other side, Ihar would you share with me 
> your  password of your email? You can believe me I won't do
> anything wrong with it.

No, I cannot. There is no trust between us. (Though I *would* share my
password with my wife though, if situation would require it.)

> 
> And "yes" I don't want to trust anybody this is huge amount of work
> to PTL.
> 

Well, if you don't trust *anybody*, then it will be hard indeed to
find any contributors for your project. But here, I think it's not ACL
to fix, but PTL attitude.

> PTL in such case is bottleneck because he need to check that all
> 100500+ subcores are reviewing pieces that they can review and
> passing +2 only on patches that they can actually merge.
> 

He wouldn't need to check all those tiny details if he would trust his
own team just a little.

There are plenty of ways to break a project, no matter how limited
your ACLs are. The good news is that people care about their
reputation and other's trust.

There are also plenty of ways to suspend velocity of your project, and
excessive ACL fences are one of those.

> 
> Let's just automate this stuff. Like we have automated CI for
> testing.
> 
> Best regards, Boris Pavlovic
> 
> On Wed, Jun 3, 2015 at 2:28 PM, Ihar Hrachyshka
> <ihrachys at redhat.com <mailto:ihrachys at redhat.com>> wrote:
> 
> On 06/03/2015 08:29 AM, Boris Pavlovic wrote:
>> Guys,
> 
>> I will try to summarize all questions and reply on them:
> 
>> *- Why not splitting repo/plugins?*
> 
>> I don't want to make "architectural" decisions based on "social"
>> or "not enough good tool for review" issues.
> 
>> If we take a look at OpenStack that was splited many times: 
>> Glance, Cinder, ... we will see that there is a log of code 
>> duplication that can't be removed even after two or even more
>> years of oslo effort. As well it produce such issues like
>> syncing requirements, requirements in such large bash script like
>> devstack, there is not std installator, it's quite hard to manage
>> and test it and so on..
> 
>> That's why I don't think that splitting repo is good 
>> "architecture" decision - it makes simple things complicated...
> 
> 
>> *- Why not just trust people*
> 
>> People get tired and make mistakes (very often).
> 
> I wouldn't say they make mistakes *too* often. And if there is a 
> mistake, we always have an option to git-revert and talk to the
> guy about it. I believe no one in the neutron team merges random
> crap, and I would expect the same from other openstack teams.
> 
> It's also quite natural that people who do more reviews extend
> their field of expertise. Do we really want to chase PTLs to
> introduce a change into turing-complete-acl-description each time
> we feel someone is now ready to start reviewing code from yet
> another submodule?
> 
> Or consider a case when a patch touches most, if not all
> submodules, but applies some very trivial changes, like a new
> graduated oslo library being consumed, or python3 adoption changes.
> Do you want to wait for a super-core with enough ACL permissions
> for all those submodules touched to approve it? I would go the
> opposite direction, allowing a single core to merge such a trivial
> patch, without waiting for the second one to waste his time
> reviewing it.
> 
> Core reviewers are not those who are able to put +2 on any patch,
> but those who are able to understand where *not* to put it. I would
> better allow people themselves to decide where they are capable and
> where their expertise ends, and free PTLs from micro-managing the
> cats.
> 
> So in essence: mistakes are cheap; reputation works; people are 
> responsible enough; and more ACL fences are evil.
> 
>> That's why we have blocking CI system that checks patches,
> 
> Those checks are easy to automate. Trust is not easily formalized
> though .
> 
> Ihar
> 
> ______________________________________________________________________
____
>
> 
OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: 
> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe 
> <http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe>
>
> 
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> 
> ______________________________________________________________________
____
>
> 
OpenStack Development Mailing List (not for usage questions)
> Unsubscribe:
> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe 
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVbu6SAAoJEC5aWaUY1u57y7AIAMcumeEWEapzT387uNNXDfz0
+nRrKPvJdgp3+lv0x7cbHv3p6kelF9E08z3Yjq4Kxw8Jc1PJkCJk2mbLAswIwZ3T
bTRaKCPQ2dDi503YJTgoTuqcZsHf8oQEfpv03AobEXWahB+0oH4aG+W16f3RdyoI
z4WtVILWv24n+MWkHLBcBddg/3/t/fcY8nsU6FElN5X+oYozeN7YF28URXgxB1I9
eU7p5ODA43YCkYlbWDIc4HicZbhIq0H3JholTqcpMrreeiDA9bBDtLEbDWmqlEp+
pVoIMj4OT7vYwIFdFzU6OLI+bwWL2f9UOkdFx+Zx/bbawj0Sp+A+1hGh4c/y82c=
=xmv/
-----END PGP SIGNATURE-----



More information about the OpenStack-dev mailing list