[openstack-dev] [Fuel] Code review process in Fuel and related issues

Mike Scherbakov mscherbakov at mirantis.com
Thu Sep 3 06:19:27 UTC 2015


Thank you all for the feedback.


Dims -

> 1) I'd advise to codify a proposal in fuel-specs under a 'policy'
directory

I think it's great idea and I'll do it.


> 2) We don't have SME terminology, but we do have Maintainers both in
oslo-incubator

I like "Maintainers" more than SMEs, thank you for suggestion. I'd switch
SME -> Maintainer everywhere.


> 3) Is there a plan to split existing repos to more repos? Then each repo
can have a core team (one core team for one repo), PTL takes care of all
repos and MAINTAINERS take care of directories within a repo. That will
line up well with what we are doing elsewhere in the community (essentially
"Component Lead" is a core team which may not be a single person).

That's the plan, with one difference though. According to you, there is a
core team per repo without a lead identified. In my proposal, I'd like to
ensure that we always choose a lead by the process of voting. I'd like to
have voting process of identifying a component lead. It has to be fair
process.


> We do not have a concept of SLA anywhere that i know of, so it will have
to be some kind of social consensus and not a real carrot/stick.

> As for me the idea of SLA contradicts to qualitative reviews.

I'm not thinking about carrot or stick here. I'd like to ensure that core
reviewers and component leads are targeted to complete reviews within a
certain time. If it doesn't happen, then patchset needs to be discussed
during IRC meeting if we can delegate some testing, etc. to someone. If
there are many patchsets out of SLA, then we'd consider other changes
(decide to drop something from the release so to free up resources or
something else).

We had a problem in the past, when we would not pay attention to a patch
proposed by someone before it's being escalated. I'm suggesting a solution
for that problem. SLA is the contract between contributor and reviewer, and
both would have same expectations on how long would it take to review the
patch. Without expectations aligned, contributors can get upset easily.
They may expect that their code will be reviewed and merged within hours,
while it fact it's days. I'm not even talking about patches which can be
forgotten and hang in the queue for months...


> If we succeed in reducing the load on core reviewers, it will mean that
core reviewers will do less code reviews. This could lead to core reviewer
demotion.

I expect that there will be a drop in code reviews being done by core
reviewers team. This is the point actually - do less reviews, but do it
more thoroughly. Don't work on patches which have easy mistakes, as those
should be cought by maintainers, before these patches come to the core
reviewer's plate. I don't think though that it will lead to core reviewer
"demotion". Still, you will be doing many reviews - just less than before,
and other who did a little - will do more reviews, potentially becoming
joining a core team later.


> It would be nice if Jenkins could add reviewers after CI +1, or we can
use gerrit dashboard for SMEs to not waste their time on review that has
not yet passed CI and does not have +1 from other reviewers.

This is good suggestion. I agree.


> AFAIK Boris Pavlovic introduced some scripts

> in Rally which do basic preliminary check of review message, checking

> that it's formally correct.

Thanks Igor, I believe this can be applied as well.


> Another thing is I got a bit confused by the difference between Core
Reviewer and Component Lead,

> aren't those the same persons? Shouldn't every Core Reviewer know the
architecture, best practises

> and participate in design architecture sessions?

Component Lead is being elected by core reviewers team as the lead. So it's
just another core reviewer / architect, but with the right to have a final
word. Also, for large parts like fuel-library / nailgun, I'd expect this
person to be free from any feature-work. For smaller things, like network
verifier, I don't think that we'd need to have dedicated component owner
who will be free from any feature work.


Igor K., Evgeny L, did I address your questions regarding SLA and component
lead vs core reviewer?

Thank you,

On Wed, Sep 2, 2015 at 9:28 AM Jay Pipes <jaypipes at gmail.com> wrote:

> On 09/02/2015 08:45 AM, Igor Kalnitsky wrote:
> >> I think there's plenty of examples of people in OpenStack projects
> >> that both submit code (and lead features) that also do code review
> >> on a daily basis.
> >
> > * Do these features huge?
>
> Yes.
>
> > * Is their code contribution huge or just small patches?
>
> Both.
>
> > * Did they get to master before FF?
>
> Yes.
>
> > * How many intersecting features OpenStack projects have under
> > development? (since often merge conflicts requires a lot of re-review)
>
> I recognize that Fuel, like devstack, has lots of cross-project
> dependencies. That just makes things harder to handle for Fuel, but it's
> not a reason to have core reviewers not working on code or non-core
> reviewers not doing reviews.
>
> > * How often OpenStack people are busy on other activities, such as
> > helping fellas, troubleshooting customers, participate design meetings
> > and so on?
>
> Quite often. I'm personally on IRC participating in design discussions,
> code reviews, and helping people every day. Not troubleshooting
> customers, though...
>
> > If so, do you sure they are humans then? :) I can only speak for
> > myself, and that's what I want to say: during 7.0 dev cycle I burned
> > in hell and I don't want to continue that way.
>
> I think you mean you "burned out" :) But, yes, I hear you. I understand
> the pressure that you are under, and I sympathize with you. I just feel
> that the situation is not an either/or situation, and encouraging some
> folks to only do reviews and not participate in coding/feature
> development is a dangerous thing.
>
> Best,
> -jay
>
> > Thanks,
> > Igor
> >
> > On Wed, Sep 2, 2015 at 3:14 PM, Jay Pipes <jaypipes at gmail.com> wrote:
> >> On 09/02/2015 03:00 AM, Igor Kalnitsky wrote:
> >>>
> >>> It won't work that way. You either busy on writing code / leading
> >>> feature or doing review. It couldn't be combined effectively. Any
> >>> context switch between activities requires an extra time to focus on.
> >>
> >>
> >> I don't agree with the above, Igor. I think there's plenty of examples
> of
> >> people in OpenStack projects that both submit code (and lead features)
> that
> >> also do code review on a daily basis.
> >>
> >> Best,
> >> -jay
> >>
> >>
> >>
> __________________________________________________________________________
> >> 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
> >
> >
> __________________________________________________________________________
> > 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
> >
>
> __________________________________________________________________________
> 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
>
-- 
Mike Scherbakov
#mihgen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150903/9cad859f/attachment.html>


More information about the OpenStack-dev mailing list