Open Stack

On Fri, Oct 17, 2014 at 2:09 PM, Joe Gordon <joe.gordon0 at gmail.com> wrote:

>
> On Thu, Oct 16, 2014 at 8:53 PM, Morgan Fainberg <
> morgan.fainberg at gmail.com> wrote:
>
>> I agree we should use flake8 built-in if at all possible. I complexity
>> checking will definitely help us in the long run keeping code maintainable.
>>
>
> Well this is scary:
>
> ./nova/virt/libvirt/driver.py:3736:1: C901 'LibvirtDriver._get_guest_config' is too complex (67)
>
> http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n373 <http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n3736>
>
> to
>
> *http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n4113 <http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n4113>*
>
>
>
>
> First step in fixing this, put a cap on it:  <http://goog_106984861>
> https://review.openstack.org/129125
>
>
Thanks for that, I mostly copied your patch for Heat:
https://review.openstack.org/#/c/129126/

I wish it would print out the value for all the modules tho', that is what
is nice about radon.

-Angus


>
>
>>
>> +1 from me.
>>
>>
>> —
>> Morgan Fainberg
>>
>>
>> On October 16, 2014 at 20:45:35, Joe Gordon (joe.gordon0 at gmail.com)
>> wrote:
>> > On Thu, Oct 16, 2014 at 8:11 PM, Angus Salkeld
>> > wrote:
>> >
>> > > Hi all
>> > >
>> > > I came across some tools [1] & [2] that we could use to make sure we
>> don't
>> > > increase our code complexity.
>> > >
>> > > Has anyone had any experience with these or other tools?
>> > >
>> >
>> >
>> > Flake8 (and thus hacking) has built in McCabe Complexity checking.
>> >
>> > flake8 --select=C --max-complexity 10
>> >
>> > https://github.com/flintwork/mccabe
>> > http://flake8.readthedocs.org/en/latest/warnings.html
>> >
>> > Example on heat: http://paste.openstack.org/show/121561
>> > Example in nova (max complexity of 20):
>> > http://paste.openstack.org/show/121562
>> >
>> >
>> > >
>> > > radon is the underlying reporting tool and xenon is a "monitor" -
>> meaning
>> > > it will fail if a threshold is reached.
>> > >
>> > > To save you the time:
>> > > radon cc -nd heat
>> > > heat/engine/stack.py
>> > > M 809:4 Stack.delete - E
>> > > M 701:4 Stack.update_task - D
>> > > heat/engine/resources/server.py
>> > > M 738:4 Server.handle_update - D
>> > > M 891:4 Server.validate - D
>> > > heat/openstack/common/jsonutils.py
>> > > F 71:0 to_primitive - D
>> > > heat/openstack/common/config/generator.py
>> > > F 252:0 _print_opt - D
>> > > heat/tests/v1_1/fakes.py
>> > > M 240:4 FakeHTTPClient.post_servers_1234_action - F
>> > >
>> > > It ranks the complexity from A (best) upwards, the command above (-nd)
>> > > says only show D or worse.
>> > > If you look at these methods they are getting out of hand and are
>> > > becoming difficult to understand.
>> > > I like the idea of having a threshold that says we are not going to
>> just
>> > > keep adding to the complexity
>> > > of these methods.
>> > >
>> > > This can be enforced with:
>> > > xenon --max-absolute E heat
>> > > ERROR:xenon:block "heat/tests/v1_1/fakes.py:240
>> post_servers_1234_action"
>> > > has a rank of F
>> > >
>> > > [1] https://pypi.python.org/pypi/radon
>> > > [2] https://pypi.python.org/pypi/xenon
>> > >
>> > > If people are open to this, I'd like to add these to the
>> test-requirements
>> > > and trial this in Heat
>> > > (as part of the pep8 tox target).
>> > >
>> >
>> > I think the idea of gating on complexity is a great idea and would like
>> to
>> > see nova adopt this as well. But why not just use flake8's built in
>> stuff?
>> >
>> >
>> > >
>> > > Regards
>> > > Angus
>> > >
>> > > _______________________________________________
>> > > OpenStack-dev mailing list
>> > > OpenStack-dev at lists.openstack.org
>> > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> > >
>> > >
>> > _______________________________________________
>> > OpenStack-dev mailing list
>> > OpenStack-dev at lists.openstack.org
>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> >
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141017/fa1467cc/attachment.html>