Open Stack

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:  <goog_106984861>
https://review.openstack.org/129125



>
> +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
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141016/adf4e8e6/attachment.html>