Open Stack

On 06/24/2015 08:42 AM, Daniel P. Berrange wrote:
> On Wed, Jun 24, 2015 at 11:28:59AM +0100, Nikola Đipanov wrote:
>> Hey Nova,
>>
>> I'll cut to the chase and keep this email short for brevity and clarity:
>>
>> Specs don't work! They do nothing to facilitate good design happening,
>> if anything they prevent it. The process layered on top with only a
>> minority (!) of cores being able to approve them, yet they are a prereq
>> of getting any work done, makes sure that the absolute minimum that
>> people can get away with will be proposed. This in turn goes and
>> guarantees that no good design collaboration will happen. To add insult
>> to injury, Gerrit and our spec template are a horrible tool for
>> discussing design. Also the spec format itself works for only a small
>> subset of design problems Nova development is faced with.
>
> I'd like to see some actual evidence to backup a sweeping statement
> as "Specs dont work. They do nothing to facilitate good design happening,
> if anything they prevent it."
>
> Comparing Nova today, with Nova before specs were introduced, I think
> that specs have had a massive positive impact on the amount of good
> design and critique that is happening.

Yes, I feel the same.

> When I was reviewing features in Nova before specs came along, I spent
> alot of time just trying to figure out what on earth the code was
> actually attempting to address, because there was rarely any statement
> of the problem being addressed, or any explanation of the design that
> motivated the code.  This made life hard for reviewers trying to figure
> out if the code was acceptable to merge.  It is pretty bad for contributors
> trying to implement new features too, as they could spend weeks or months
> writing and submitting code, only to be rejected at the end because the
> (lack of any design discussions) meant they missed some aspect of the
> problem which in turn meant all their work was in vain. That was a collosal
> waste of everyone's time and resulted in some of the very horrible code
> impl decisions we're still living with today.

Right.

<snip>

> In reviewing specs, we also often get into far too much detail about
> the actual implementation, which is really counterproductive. eg when
> get down to bikeshedding about the names of classes, types of object
> attributes, and so forth, it is really insane. That level of detail
> and bikesheeding belongs in the code review, not the design for the
> most part.

While we can sometimes get a little deep in implementation details, I 
have one comment about this.

Naming is *extremely* important. I know it sounds silly, but what people 
name classes and modules and methods goes directly to the readability 
and understandability of the code itself. I often bring up naming points 
in spec reviews because I feel it forces the submitter to think through 
how to explain their proposed solution to people. I know that 
personally, conversations about naming things in my own submitted spec 
reviews have been immensely useful. As an example recently, Alexis Lee 
and Ed Leafe had some comments about how I'd named the objects in my 
resource-objects spec, and although I went back and forth with Alexis on 
a number of things, the end result I am confident produced objects and 
methods that were named in a way that is most readable and 
comprehensible to contributors.

> Specs are also inflexible when it comes to dealing with features that
> cross multiple projects, because we silo'd spec reviews against the
> individual projects. This is sub-optimal, but ultimately not a show
> stopper.

Well, we do have the openstack-specs repo for cross-project specs, but 
it's not well reviewed, to be fair.

> I also think the way we couple spec approval & reviews to the dev
> cycles is counterproductive. We should be willing to accept and
> review specs at any point in any cycle, and once approved they should
> remain valid for a prolonged period of time - not require us to go
> through re-review every new dev cycle as again that's just creating
> extra burden. We should of course though reserve the right to unapprove
> specs if circumstances change, invlidating the design for the previous
> approval.

OMG, thank you SO much for bringing this up. I could not agree more with 
the above paragraph. We can always amend a previously approved spec with 
additional information based on followup conversations, but taking the 
time to continually push new specs for review each cycle is a colossal 
waste of time.

Best,
-jay