Open Stack

On 06/25/2015 09:46 PM, Joe Gordon wrote:
> On Thu, Jun 25, 2015 at 1:39 AM, Nikola Đipanov <ndipanov at redhat.com
>  
> 
> 
>     > As someone who does a lot of spec reviews, I take +1s from the right
>     > people (not always nova-cores) to mean a lot, so much that I regularly
>     > will simply skim the spec myself before +2ing it. If a subject matter
>     > expert who I trust +1s a spec, that is usually all I need.
>     >
>     > * +1/-1s from the right people have a lot of power on specs. So the
>     > review burden isn't just on the people with '+W' power.  We may not have
>     > done a great job of making this point clear.
>     > * There are many subject matter experts outside nova-core who's vote
>     > means a lot. For example PTL's of other projects the spec impacts.
>     >
> 
>     This is exactly the kind of cognitive dissonance I find hard to not get
>     upset about :)
> 
>     Code is what matters ultimately - the devil _is_ in the details, and I
>     can bet you that it is extremely unlikely that a PTL of any other
>     project is also going to go and do a detailed review of a feature branch
>     in Nova, and have the understanding of the state of the surrounding
>     codebase needed to do it properly. That's what's up to the nova
>     reviewers to deal with. I too wish Nova code was in a place where it was
>     possible to just do "architecture", but I think we all agree it's
>     nowhere near that.
> 
> 
> This goes back to the point(s) that was brought up over and over again
> in this thread. I guess we have to agree to disagree.
> 
> I think saying 'code' is what ultimately matters is misleading.  'Code'
> is the implementation of an idea. If an idea is bad, so what if the code
> is good?
> 
> I wouldn't ask the PTL of say Keystone to review the implementation of
> some idea in nova, but I do want their opinion on an idea that impacts
> how nova and keystone interact. Why make someone write a bunch of code,
> only to find out that the design itself was fundamentally flawed and
> they have to go back to the drawing board and throw everything out. On
> top of that now the reviewers has to mentally decouple the idea and the
> code (unless the feature has good documentation explaining that -- sort
> of like a spec).
> 
> That being said, I do think there is definitely room for improvement.
> 

It really goes both way - it's important to state the problem and the
general direction the implementation plans to take, but anything more
than that is a distraction from getting to a prototype that will tell us
more about if the design was in fact fundamentally flawed. We have
examples of that in tree right now - stuff was written and re-written
and got better. Spec will never remove the need for that and we should
not try to make them.

Also "throwing code away" is a bit of a straw-man, good code will almost
never be thrown out entirely, some bits of it - sure (that's software) -
you may change an interface to a class, or a DB schema detail here and
there - but if your code is written to be modular and does not leak
abstractions - you'll end up keeping huge bits of it through rewrites.
We need more of that!

> 
>     With all due respect to you Joe (and I do have a lot of respect for you)
>     - I can't get behind how Nova specs puts process and documents over
>     working and maintainable code. I will never be able to get behind that!
> 
> 
> 
> So what are you proposing ultimately?  It sounds like the broad
> consensus here is: specs have made things better, but there is room for
> improvement (this is my opinion as well). Are you saying just drop specs
> all together? Because based on the discussion here, there isn't anything
> near consensus for doing that. So if we aren't going to just revert to
> how things were before specs, what do you think we should do?
> 

I will follow up with a more detailed email, but in short - acknowledge
that some problems are fundamentally different than others, decide what
kind of work absolutely requires an up front discussion (API seems like
a solid candidate) and drop the blanket requirement for a detailed spec
for any work (do still require a problem statement though, maybe in a
lighter format as part of the branch).

A lot of it comes back to our release mechanism too, and is definitely
something we need to work on incrementally.

N.