Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/16/2014 11:09 PM, Joe Gordon wrote: > First step in fixing this, put a cap on it: > https://review.openstack.org/129125 To get the maximums down to more reasonable levels, I have two patches that bring the libvirt driver _get_guest_config() fro

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

I would also advise pinning the version of mccabe we’re using. Mccabe was originally a proof-of-concept script that Ned Batchelder wrote and which Tarek Ziade vendored into Flake8. After we split it out in v2 of Flake8, we’ve found several (somewhat serious) reporting problems with the tool. Curren

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On 10/17/2014 05:10 AM, Chris Dent wrote: On Fri, 17 Oct 2014, Daniel P. Berrange wrote: IMHO this tool is of pretty dubious value. I mean that function is long for sure, but it is by no means a serious problem in the Nova libvirt codebase. The stuff it complains about in the libvirt/config.py

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, 17 Oct 2014, Daniel P. Berrange wrote: IMHO this tool is of pretty dubious value. I mean that function is long for sure, but it is by no means a serious problem in the Nova libvirt codebase. The stuff it complains about in the libvirt/config.py file is just incredibly stupid thing to hig

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

I like measuring code metrics, and I definitely support Joe's change here. I think of McCabe complexity as a proxy for testability and readability of code, both of which are IMO actual real problems in the nova codebase. If you are an experienced openstack dev you might find the code easy to move a

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, Oct 17, 2014 at 03:03:43PM +1100, Michael Still wrote: > I think nova wins. We have: > > ./nova/virt/libvirt/driver.py:3736:1: C901 > 'LibvirtDriver._get_guest_config' is too complex (67) IMHO this tool is of pretty dubious value. I mean that function is long for sure, but it is by no mea

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

Excerpts from Mike Spreitzer's message of 2014-10-16 22:24:30 -0700: > I like the idea of measuring complexity. I looked briefly at `python -m > mccabe`. It seems to measure each method independently. Is this really > fair? If I have a class with some big methods, and I break it down into >

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, Oct 17, 2014 at 2:39 PM, Joe Gordon wrote: > > First step in fixing this, put a cap on it: > https://review.openstack.org/129125 > Thanks Joe - I've just put up a similar patch for Ironic: https://review.openstack.org/129132 -- Michael Davies mich...@the-davi

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, Oct 17, 2014 at 3:24 PM, Mike Spreitzer wrote: > I like the idea of measuring complexity. I looked briefly at `python -m > mccabe`. It seems to measure each method independently. Is this really > fair? I think it is a good starting point. You still need to write methods that do one t

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

I like the idea of measuring complexity. I looked briefly at `python -m mccabe`. It seems to measure each method independently. Is this really fair? If I have a class with some big methods, and I break it down into more numerous and smaller methods, then the largest method gets smaller, but

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On 17/10/14 15:23, Angus Salkeld wrote: > 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. It's a little horrible, but we can do the same thing: (

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, Oct 17, 2014 at 2:09 PM, Joe Gordon wrote: > > On Thu, Oct 16, 2014 at 8:53 PM, Morgan Fainberg < > morgan.fainb...@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.

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Thu, Oct 16, 2014 at 10:09 PM, Joe Gordon wrote: > > On Thu, Oct 16, 2014 at 8:53 PM, Morgan Fainberg < > morgan.fainb...@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.

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Thu, Oct 16, 2014 at 8:53 PM, Morgan Fainberg 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_gue

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

On Fri, Oct 17, 2014 at 1:40 PM, Joe Gordon 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 t

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

I think nova wins. We have: ./nova/virt/libvirt/driver.py:3736:1: C901 'LibvirtDriver._get_guest_config' is too complex (67) Michael On Fri, Oct 17, 2014 at 2:45 PM, Dolph Mathews wrote: > I ran this tool on Keystone and liked the results - the two methods that > ranked D should certainly be re

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

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. +1 from me.  — Morgan Fainberg On October 16, 2014 at 20:45:35, Joe Gordon (joe.gord...@gmail.com) wrote: > On Thu, Oct 16, 2014 at 8:11 PM, Angus

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

I ran this tool on Keystone and liked the results - the two methods that ranked D should certainly be refactored. It also matched a few methods in openstack/common. But flake8 supports complexity checking already, using mccabe. Just enable complexity checking with: $ flake8 --max-complexity 20

Re: [openstack-dev] [all] add cyclomatic complexity check to pep8 target

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 c