[openstack-dev] [nova] RFC - using Gerrit for Nova Blueprint review & approval

Murray, Paul (HP Cloud Services) pmurray at hp.com
Fri Mar 7 13:57:15 UTC 2014


The principle is excellent, I think there are two points/objectives worth keeping in mind:

1. We need an effective way to make and record the design decisions
2. We should make the whole development process easier

In my mind the point of the design review part is to agree up front something that should not be over-turned (or is hard to over-turn) late in patch review. I agree with others that a patch should not be blocked (or should be hard to block) because the reviewer disagrees with an agreed design decision. Perhaps an author can ask for a -2 or -1 to be removed if they can point out the agreed design decision, without having to reopen the debate. 

I also think that blueprints tend to have parts that should be agreed up front, like changes to apis, database migrations, or integration points in general. They also have parts that don't need to be agreed up front, there is no point in a heavyweight process for everything. Some blueprints might not need any of this at all. For example, a new plugin for the filter scheduler might no need a lot of design review, or at least, adding the design review is unlikely to ease the development cycle.

So, we could use the blueprint template to identify things that need to be agreed in the design review. These could include anything the proposer wants agreed up front and possibly specifics of a defined set of integration points. Some blueprints might have nothing to be formally agreed in design review. Additionally, sometimes plans change, so it should be possible to return to design review. Possibly the notion of a design decision could be broken out form a blueprint in the same way as a patch-set? maybe it only makes sense to do it as a whole? Certainly design decisions should be made in relation to other blueprints and so it should be easy to see that there are two blueprints making related design decisions.

The main point is that there should be an identifiable set of design decisions that have reviewed and agreed that can also be found.

**The reward for authors in doing this is the author can defend their patch-set against late objections to design decisions.**
**The reward for reviewers is they get a way to know what has been agreed in relation to a blueprint.**

On another point...
...sometimes I fall foul of writing code using an approach I have seen in the code base, only to be told it was decided not to do it that way anymore. Sometimes I had no way of knowing that, and exactly what has been decided, when it was decided, and who did the deciding has been lost. Clearly the PTL and ML do help out here, but it would be helpful if such things were easy to find out. These kinds of design decision should be reviewed and recorded.

Again, I think it is excellent that this is being addressed.

Paul.



-----Original Message-----
From: Sean Dague [mailto:sean at dague.net] 
Sent: 07 March 2014 12:01
To: OpenStack Development Mailing List (not for usage questions)
Subject: Re: [openstack-dev] [nova] RFC - using Gerrit for Nova Blueprint review & approval

On 03/07/2014 06:30 AM, Thierry Carrez wrote:
> Sean Dague wrote:
>> One of the issues that the Nova team has definitely hit is Blueprint 
>> overload. At some point there were over 150 blueprints. Many of them 
>> were a single sentence.
>>
>> The results of this have been that design review today is typically 
>> not happening on Blueprint approval, but is instead happening once 
>> the code shows up in the code review. So -1s and -2s on code review 
>> are a mix of design and code review. A big part of which is that 
>> design was never in any way sufficiently reviewed before the code started.
>>
>> In today's Nova meeting a new thought occurred. We already have 
>> Gerrit which is good for reviewing things. It gives you detailed 
>> commenting abilities, voting, and history. Instead of attempting (and 
>> usually
>> failing) on doing blueprint review in launchpad (or launchpad + an 
>> etherpad, or launchpad + a wiki page) we could do something like follows:
>>
>> 1. create bad blueprint
>> 2. create gerrit review with detailed proposal on the blueprint 3. 
>> iterate in gerrit working towards blueprint approval 4. once approved 
>> copy back the approved text into the blueprint (which should now be 
>> sufficiently detailed)
>>
>> Basically blueprints would get design review, and we'd be pretty sure 
>> we liked the approach before the blueprint is approved. This would 
>> hopefully reduce the late design review in the code reviews that's 
>> happening a lot now.
>>
>> There are plenty of niggly details that would be need to be worked 
>> out
>>
>>  * what's the basic text / template format of the design to be 
>> reviewed (probably want a base template for folks to just keep things consistent).
>>  * is this happening in the nova tree (somewhere in docs/ - NEP (Nova 
>> Enhancement Proposals), or is it happening in a separate gerrit tree.
>>  * are there timelines for blueprint approval in a cycle? after which 
>> point, we don't review any new items.
>>
>> Anyway, plenty of details to be sorted. However we should figure out 
>> if the big idea has support before we sort out the details on this one.
>>
>> Launchpad blueprints will still be used for tracking once things are 
>> approved, but this will give us a standard way to iterate on that 
>> content and get to agreement on approach.
> 
> Sounds like an interesting experiment, and a timely one as we figure 
> out how to do blueprint approval in the future with StoryBoard.
> 
> I'm a bit skeptical that can work without enforcing that changes 
> reference at least a bug or a blueprint, though. People who were too 
> lazy to create a single-sentence blueprint to cover for their feature 
> will definitely not go through a Gerrit-powered process, so the 
> temptation to fly your smallish features below the radar ("not worth 
> this whole blueprint approval thing") and just get them merged will be 
> high. I fear it will overall result in work being less tracked, rather 
> than more tracked.
> 
> FWIW we plan to enforce a bug reference / blueprint reference in 
> changes with StoryBoard, but it comes with autocreation of missing 
> bugs/blueprints (from the commit message) to lower the developer hassle.
> 
> That being said, don't let my skepticism go into the way of your 
> experimentation. We definitely need to improve in this area. I'd like 
> to have a cross-project session on feature planning/tracking at the 
> Design Summit, where we can brainstorm more ideas around this.

Honestly, right now we're not trying to fix all things (or enforce all things). We're trying to fix a very specific issue that because we are tool-failing on blueprint approval, as it's entirely impossible to have a detailed conversation in launchpad, we're failing open with a bunch of approved and targeted blueprints that no one understands what they are.

I want StoryBoard more than anyone else. However future Puppies and Unicorns don't fix real problems right now. With the tools already at our disposal, just using them a different way, I think we can fix some real problems. I think, more importantly, we're going to discover a whole new class of problems because we're not blocked on launchpad.

And the fact that the Nova team and the Ops team came up with the same idea, independently, within a week of each other, is a reasonable indication that it's worth trying. Because it seriously can't be worse than the current model. :)

	-Sean

--
Sean Dague
Samsung Research America
sean at dague.net / sean.dague at samsung.com
http://dague.net



More information about the OpenStack-dev mailing list