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

Boris Pavlovic boris at pavlovic.me
Wed Jun 3 13:43:14 UTC 2015


Julien,

If I were on you shoes I would pick words more carefully.

When you are saying:

> Reverting patches is unacceptable for Rally project.
> Then you have a more serious problem than the rest of OpenStack.


"you" means Rally community which is quite large.
http://stackalytics.com/?release=liberty&metric=commits&project_type=openstack&module=rally


And I don't understand "what" so serious problem we have.
We were not able to do reverts so  we build CI that doesn't allow us to
break master
 so we don't need to do reverts. I really don't see here any big problems.

> This means that we merged bug and this is epic fail of PTL of project.
> Your code is already full of bugs and misfeatures, like the rest of the
> software. That's life.


I was talking about reverting patches. And I believe the process is broken
if you need to revert patches. It means that core team is not enough team
or CI is not enough good.


If you're having trust issues, good luck maintaining any large-scale
> successful (open source) project. This is terrible management and leads
> to micro-managing tasks and people, which has never build something
> awesome.


I don't believe even my self, because I am human and I make mistakes.
My goal on the PTL position is to make such process that stops "human"
mistakes before they land in master. In other words  everything should be
automated and pre not post checked.

Best regards,
Boris Pavlovic

On Wed, Jun 3, 2015 at 4:00 PM, Thierry Carrez <thierry at openstack.org>
wrote:

> So yeah, that's precisely what we discussed at the cross-project
> workshop about In-team scaling in Vancouver (led by Kyle and myself).
> For those not present, I invite you to read the notes:
>
> https://etherpad.openstack.org/p/liberty-cross-project-in-team-scaling
>
> The conclusion was to explore splitting review areas and building trust
> relationships. Those could happen:
>
> - along architectural lines (repo splits)
> - along areas of expertise with implicit trust to not review anything else
>
> ... which is precisely what you seem to oppose.
>
> Boris Pavlovic wrote:
> > *- 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...
>
> I know we disagree on that one, but I don't think monolithic means
> "simpler". Having smaller parts that have a simpler role and explicit
> contracts to communicate with other pieces is IMHO better and easier to
> maintain.
>
> We shouldn't split repositories when it only results in code
> duplication. But whenever we can isolate something that could have a
> more dedicated maintenance team, I think that's worth exploring as a
> solution to the review scaling issue.
>
> > *- Why not just trust people*
> >
> > People get tired and make mistakes (very often).
> > That's why we have blocking CI system that checks patches,
> > That's why we have rule 2 cores / review (sometimes even 3,4,5...)...
>
> It's not because "we don't trust people" that we have the 2-core rule.
> Core reviewers check the desirability and quality of implementation. By
> default we consider that if 2 of those agree that a change is sane, it
> probably is. The CI system checks something else, and that is that you
> don't break everyone or introduce a regression. So you shouldn't be able
> to "introduce a bug" that would be so serious that a simple revert would
> still be embarrassing. If you can, then you should work on your tests.
>
> I think it's totally fine to give people the ability to +2/approve
> generally, together with limits on where they are supposed to use that
> power. They will be more careful as to what they approve this way. For
> corner cases you can revert.
>
> As an example, Ubuntu development has worked on that trust model for
> ages. Once you are a developer, you may commit changes to any package in
> the distro. But you're supposed to use that power wisely. You end up
> staying away from risky packages and everything you don't feel safe to
> approve.
>
> If you can't trust your core reviewers to not approve things that are
> outside their comfort zone, I'd argue they should not be core reviewers
> in the first place.
>
> --
> Thierry Carrez (ttx)
>
> __________________________________________________________________________
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150603/9cfb7d30/attachment.html>


More information about the OpenStack-dev mailing list