[openstack-dev] [Fuel] Code review process in Fuel and related issues
Evgeniy L
eli at mirantis.com
Mon Sep 28 17:47:56 UTC 2015
Hi Mike, thanks, now it's clear.
On Thu, Sep 3, 2015 at 9:19 AM, Mike Scherbakov <mscherbakov at mirantis.com>
wrote:
> 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
>
> __________________________________________________________________________
> 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/20150928/3fa40a63/attachment.html>
More information about the OpenStack-dev
mailing list