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

Ihar Hrachyshka ihrachys at redhat.com
Wed Jun 3 11:28:01 UTC 2015


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

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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVbuS9AAoJEC5aWaUY1u57v2wH/iDLvCrebTtTpocZ8a0BFJ7T
ssgjM+1F2JiEuieNg7qRqkdW8fZuMuODc7EnWihjDjfP4OMQkelO2711KSPTCSmT
76RLMQrSHhyB2FO29qu+4bE5uwUV4uutaDyK8IRZpra+nrSoU8dtL6NuTa/csEeU
QbmJBB2UMSXdrQmA6HfzoQV9Dmqk5ePbjzg1HXTFy/AtxCb2DLf2IUmeHqwtqg1o
WoC5ISqoUkRzWx5h1IbV26hhJuGrW6pWjrX50UEFmR/VZwz9T13s7BVE4ReE7mnA
2cIGdFnhaJY/VzD4WEzXRfNXV0qetTJG6w30wktKq6y1mG6q8nm+N6KQ4Onq0FQ=
=DZSF
-----END PGP SIGNATURE-----



More information about the OpenStack-dev mailing list