[openstack-dev] [nova] An analysis of code review in Nova

Tracy Jones tjones at vmware.com
Tue Mar 25 01:27:26 UTC 2014


I agree that the oslo integration does not depend on the spawn reflector and we have been asked to do this before doing anything else by the core team.  I would like to get the spawn refactor in ASAP - as mentioned in the BP

1.  pull out the inner methods and add tests for them
2.  fix the complex branching

https://blueprints.launchpad.net/nova/+spec/vmware-spawn-refactor

I believe these 3 items are under work already by Shawn, Vui, Gary, and Ryan.


The the oslo integration can happen immediately after - or even in parallel, but i think it will go easier once the spawn refactor is complete.


On Mar 24, 2014, at 11:24 AM, Shawn Hartsock <hartsock at acm.org> wrote:

> I fully support https://urldefense.proofpoint.com/v1/url?u=https://review.openstack.org/%23/c/70175/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=fysbO8%2FBLtC%2B0WXqPRtZjP%2BFTxUY74FYnj8tkYiMlD4%3D%0A&m=kdu06OR%2BhUFnE048i3EIYluW%2FTZVQ2x8i%2FgCgVHF9U4%3D%0A&s=ecab500da1e8acaeaceecfd2df28c2c7ab6a82725202a4fecbc3b04d949b3a62 but I fail to
> see why the spawn-refactor should depend on that. There are only 13
> lines touched that are related. These two tasks could be completed
> more or less in parallel.
> 
> On Mon, Mar 24, 2014 at 4:57 AM, Gary Kotton <gkotton at vmware.com> wrote:
>> Regarding the spawn there are a number of patches up for review at the
>> moment - they are all mutually exclusive and hopefully will make the
>> process a lot smoother.
>> https://urldefense.proofpoint.com/v1/url?u=https://review.openstack.org/%23/q/topic:bp/vmware-spawn-refactor%2Cn%2Cz&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=fysbO8%2FBLtC%2B0WXqPRtZjP%2BFTxUY74FYnj8tkYiMlD4%3D%0A&m=kdu06OR%2BhUFnE048i3EIYluW%2FTZVQ2x8i%2FgCgVHF9U4%3D%0A&s=91ea5fa587f87ee0e8cd7993ec9b5deba54c1d67f6e4c261ba6bf8df0921fb94
>> In addition to this we have a patch up for review with the OSLO
>> integration - https://urldefense.proofpoint.com/v1/url?u=https://review.openstack.org/%23/c/70175/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=fysbO8%2FBLtC%2B0WXqPRtZjP%2BFTxUY74FYnj8tkYiMlD4%3D%0A&m=kdu06OR%2BhUFnE048i3EIYluW%2FTZVQ2x8i%2FgCgVHF9U4%3D%0A&s=ecab500da1e8acaeaceecfd2df28c2c7ab6a82725202a4fecbc3b04d949b3a62 (ideally it would be
>> best that this gets in first)
>> Thanks
>> Gary
>> 
>> On 3/22/14 8:03 PM, "Chris Behrens" <cbehrens at codestud.com> wrote:
>> 
>>> I'd like to get spawn broken up sooner rather than later, personally. It
>>> has additional benefits of being able to do better orchestration of
>>> builds from conductor, etc.
>>> 
>>> On Mar 14, 2014, at 3:58 PM, Dan Smith <dms at danplanet.com> wrote:
>>> 
>>>>> Just to answer this point, despite the review latency, please don't be
>>>>> tempted to think one big change will get in quicker than a series of
>>>>> little, easy to review, changes. All changes are not equal. A large
>>>>> change often scares me away to easier to review patches.
>>>>> 
>>>>> Seems like, for Juno-1, it would be worth cancelling all non-urgent
>>>>> bug fixes, and doing the refactoring we need.
>>>>> 
>>>>> I think the aim here should be better (and easier to understand) unit
>>>>> test coverage. Thats a great way to drive good code structure.
>>>> 
>>>> Review latency will be directly affected by how good the refactoring
>>>> changes are staged. If they are small, on-topic and easy to validate,
>>>> they will go quickly. They should be linearized unless there are some
>>>> places where multiple sequences of changes make sense (i.e. refactoring
>>>> a single file that results in no changes required to others).
>>>> 
>>>> As John says, if it's just a big "change everything" patch, or a ton of
>>>> smaller ones that don't fit a plan or process, then it will be slow and
>>>> painful (for everyone).
>>>> 
>>>>> +1 sounds like a good first step is to move to oslo.vmware
>>>> 
>>>> I'm not sure whether I think that refactoring spawn would be better done
>>>> first or second. My gut tells me that doing spawn first would mean that
>>>> we could more easily validate the oslo refactors because (a) spawn is
>>>> impossible to follow right now and (b) refactoring it to smaller methods
>>>> should be fairly easy. The tests for spawn are equally hard to follow
>>>> and refactoring it first would yield a bunch of more unit-y tests that
>>>> would help us follow the oslo refactoring.
>>>> 
>>>> However, it sounds like the osloificastion has maybe already started and
>>>> that refactoring spawn will have to take a backseat to that.
>>>> 
>>>> --Dan
>>>> 
>>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.org
>>>> 
>>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.openstack.org/cgi
>>>> -bin/mailman/listinfo/openstack-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r
>>>> =eH0pxTUZo8NPZyF6hgoMQu%2BfDtysg45MkPhCZFxPEq8%3D%0A&m=q5RejnjmrSIFg0K7ua
>>>> AZbKHVqAKLHnVAB98J%2BszOfhw%3D%0A&s=1629f4e9008260c5f8ea577da1bdc69388dbe
>>>> fa3646803244df992a31d94bc96
>>> 
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.openstack.org/cgi-
>>> bin/mailman/listinfo/openstack-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=e
>>> H0pxTUZo8NPZyF6hgoMQu%2BfDtysg45MkPhCZFxPEq8%3D%0A&m=q5RejnjmrSIFg0K7uaAZb
>>> KHVqAKLHnVAB98J%2BszOfhw%3D%0A&s=1629f4e9008260c5f8ea577da1bdc69388dbefa36
>>> 46803244df992a31d94bc96
>> 
>> 
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> https://urldefense.proofpoint.com/v1/url?u=http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=fysbO8%2FBLtC%2B0WXqPRtZjP%2BFTxUY74FYnj8tkYiMlD4%3D%0A&m=kdu06OR%2BhUFnE048i3EIYluW%2FTZVQ2x8i%2FgCgVHF9U4%3D%0A&s=d41dda424843e3cb894aae83feda5e3fc28fe781564c1765a6493b1491df36da
> 
> 
> 
> -- 
> # Shawn.Hartsock - twitter: @hartsock - plus.google.com/+ShawnHartsock
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=fysbO8%2FBLtC%2B0WXqPRtZjP%2BFTxUY74FYnj8tkYiMlD4%3D%0A&m=kdu06OR%2BhUFnE048i3EIYluW%2FTZVQ2x8i%2FgCgVHF9U4%3D%0A&s=d41dda424843e3cb894aae83feda5e3fc28fe781564c1765a6493b1491df36da



More information about the OpenStack-dev mailing list