On 29 October 2013 03:24, John Garbutt <j...@johngarbutt.com> wrote: >> 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.
I could certainly follow that algorithm, but the implication is that all changes in behaviour are meant to go through careful design review. Right now that certainly doesn't happen - and the blueprint page also says 'major feature'. The algorithm you've put forward also suggests that a blueprint is never needed unless there is a new API call/change in behaviour/config setting - and I'd disagree with that, because design review and approval is useful in major works (such as behaviour preserving refactorings that change the DB schema/message queue utilisation etc - not externally visible but still important to get right). -> I think 'bug vs feature' is the wrong language for framing the problem we are trying to solve. Here are my assumptions: - We have rigorous code review by folk who are a) familiar with the domain b) familiar with the skeletons and c) familiar with project history This will catch [most] poor works, whether large or small, at review time. - we want to respect the effort of our contributors and offer a means to steer their work into good designs and avoid problematic designs before significant development effort is invested. [Lets define significant as 2 days coding]. - we want to be able to say record that we said 'no' to some ideas and reference that if they come up again - the same people doing code review are those doing design review, more or less - latency on design review will be worse than that on code review (because written code is inventory, and we should be aiming to land it and avoid rebase hell) Any minor development effort - whether it includes a new config setting, API call, or new behaviour - doesn't run the risk of significant investment, so I think it's entirely appropriate to catch that through code review. Catching it through code review will have less latency that waiting for design review, and code review includes design review anyway. Any major development effort - whether it's a large scale refactoring of unit tests, adding new functionality for users, splitting an existing thing out, adding a new backend - *anything* that is more than [significant effort cutoff point] - is something that will benefit from design review: failing to get design review increases the chance of bad things: - the patch being rejected as a bad idea - the approach needing rework [vs code level 'this would be better' requests] - reviewers disagreeing about the design and stalling the project Design review addresses these problems by a) identifying bad ideas up front, b) vetting the approach via high level walk throughs with experienced reviewers in the project and c) forming consensus amongst the -core reviews about the upcoming work. So - to me - we should focus on the size of the change, not the nature of the change, when talking about 'does it need a blueprint'. -Rob -- Robert Collins <rbtcoll...@hp.com> Distinguished Technologist HP Converged Cloud _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
