[openstack-dev] [Nova] Blueprint review process
John Garbutt
john at johngarbutt.com
Mon Oct 28 14:24:44 UTC 2013
On 25 October 2013 10:18, Joe Gordon <joe.gordon0 at gmail.com> wrote:
> On Oct 24, 2013 9:14 PM, "Robert Collins" <robertc at robertcollins.net> wrote:
>> On 24 October 2013 04:33, Russell Bryant <rbryant at redhat.com> wrote:
>> > Greetings,
>> >
>> > At the last Nova meeting we started talking about some updates to the
>> > Nova blueprint process for the Icehouse cycle. I had hoped we could
>> > talk about and finalize this in a Nova design summit session on "Nova
>> > Project Structure and Process" [1], but I think we need to push forward
>> > on finalizing this as soon as possible so that it doesn't block current
>> > work being done.
>>
>> Cool
>>
>> > Here is a first cut at the process. Let me know what you think is
>> > missing or should change. I'll get the result of this thread posted on
>> > the wiki.
>> >
>> > 1) Proposing a Blueprint
>> >
>> > Proposing a blueprint for Nova is not much different than other
>> > projects. You should follow the instructions here:
>> >
>> > https://wiki.openstack.org/wiki/Blueprints
>> >
>> > The particular important step that seems to be missed by most is:
>> >
>> > "Once it is ready for PTL review, you should set:
>> >
>> > Milestone: Which part of the release cycle you think your work will be
>> > proposed for merging."
>> >
>> > That is really important. Due to the volume of Nova blueprints, it
>> > probably will not be seen until you do this.
>>
>> The other thing I'm seeing some friction on is 'significant features'
>> : it sometimes feels like folk are filing blueprints for everything
>> that isn't 'the code crashed' style problems, and while I appreciate
>> folk wanting to work within the system, blueprints are a heavyweight
>> tool, primarily suited for things that require significant
>> coordination.
>>
>> > 2) Blueprint Review Team
>> >
>> > Ensuring blueprints get reviewed is one of the responsibilities of the
>> > PTL. However, due to the volume of Nova blueprints, it's not practical
>> > for me to do it alone. A team of people (nova-drivers) [2], a subset of
>> > nova-core, will be doing blueprint reviews.
>>
>> Why a subset of nova-core? With nova-core defined as 'knows the code
>> well *AND* reviews a lot', I can see that those folk are in a position
>> to spot a large class of design defects. However, there are plenty of
>> folk with expertise in e.g. SOA, operations, deployment @ scale, who
>> are not nova-core but who will spot plenty of issues. Is there some
>> way they can help out?
>>
>> > By having more people reviewing blueprints, we can do a more thorough
>> > job and have a higher quality result.
>> >
>> > Note that even though there is a nova-drivers team, *everyone* is
>> > encouraged to participate in the review process by providing feedback on
>> > the mailing list.
>>
>> I'm not sure about this bit here: blueprints don't have the spec
>> content, usually thats in an etherpad; etherpads are editable by
>> everyone - wouldn't it be better to keep the conversation together? I
>> guess part of my concern here comes back to the (ab)use of blueprints
>> for shallow features.
>>
>> > 3) Blueprint Review Criteria
>> >
>> > Here are some things that the team reviewing blueprints should look for:
>> >
>> > The blueprint ...
>> >
>> > - is assigned to the person signing up to do the work
>> >
>> > - has been targeted to the milestone when the code is
>> > planned to be completed
>> >
>> > - is an appropriate feature for Nova. This means it fits with the
>> > vision for Nova and OpenStack overall. This is obviously very
>> > subjective, but the result should represent consensus.
>> >
>> > - includes enough detail to be able to complete an initial design
>> > review before approving the blueprint. In many cases, the design
>> > review may result in a discussion on the mailing list to work
>> > through details. A link to this discussion should be left in the
>> > whiteboard of the blueprint for reference. This initial design
>> > review should be completed before the blueprint is approved.
>> >
>> > - includes information that describes the user impact (or lack of).
>> > Between the blueprint and text that comes with the DocImpact flag [3]
>> > in commits, the docs team should have *everything* they need to
>> > thoroughly document the feature.
>>
>> I'd like to add:
>> - has an etherpad with the design (the blueprint summary has no
>> markup and is a poor place for capturing the design).
>>
>> > Once the review has been complete, the blueprint should be marked as
>> > approved and the priority should be set. A set priority is how we know
>> > from the blueprint list which ones have already been reviewed.
>>
>>
>> > 4) Blueprint Prioritization
>> >
>> > I would like to do a better job of using priorities in Icehouse. The
>> > priority field services a couple of purposes:
>> >
>> > - helps reviewers prioritize their time
>> >
>> > - helps set expectations for the submitter for how reviewing this
>> > work stacks up against other things
>> >
>> > In the last meeting we discussed an idea that I think is worth trying at
>> > least for icehouse-1 to see if we like it or not. The idea is that
>> > *every* blueprint starts out at a Low priority, which means "best
>> > effort, but no promises". For a blueprint to get prioritized higher, it
>> > should have 2 nova-core members signed up to review the resulting code.
>> >
>> > If we do this, I suspect we may end up with more blueprints at Low, but
>> > I also think we'll end up with a more realistic list of blueprints. The
>> > reality is if a feature doesn't have reviewers agreeing to do the
>> > review, it really is in a "best effort, but no promises" situation.
>>
>> I think this makes a lot of sense. Its the same basic triage process
>> we're using in the TripleO Kanban experiment - things that aren't in
>> the project current roadmap don't get unlimited resources; some things
>> are declined, and things in the roadmap everyone in the team comes
>> together to ensure a timely, effective delivery. The difference is
>> that we're operating with a deliberate overlap between folk's effort -
>> keeping the concurrent topics being worked on low, sacrificing a
>> little output to duplicate effort, but gaining a whole lot of
>> velocity.
>>
>> Joe: I disagree about merging patches with not-"approved" blueprints.
>> They are no worse than patches that don't have a blueprint.
>
> I based that on this statement, which I think sums it up well "If the patch
> implements a feature, it should reference a blueprint. The blueprint should
> be approved before the patch is merged"
>
> https://wiki.openstack.org/wiki/ReviewChecklist
>
> This does raise the question of what is consisted a feature though.
Here is a really bad attempt at codifying what I think about features vs bugs:
1) If its a new API call, or a change in behaviour, or a new config
setting, its a feature
2) If its just refactoring or just adding tests, it is neither a
feature or a bug
4) A bug is a change to ensure the system operates "as expected" by
the current documentation
3) A bug should be changed to a feature if it matches case (1)
If we don't approve the blueprint first, we may end up not having
enough information to create the required documentation, so I vote we
enforce that a blueprint should be approved before we merge code.
Getting a blueprint approved as low priority, should be quicker and
easier than getting the associated code approved. Granted that might
not be the case today, but we need to fix that.
John
More information about the OpenStack-dev
mailing list