On 12/19/2015 07:37 AM, Russell Bryant wrote: > On 12/18/2015 06:16 PM, Justin Pettit wrote: >> >>> On Dec 18, 2015, at 12:12 PM, Russell Bryant <russ...@ovn.org> wrote: >>> >>> I'm only submitting the cover letter for this RFC. The current state of >>> the patches can be seen on github. >>> >>> https://github.com/openvswitch/ovs/compare/master...russellb:python >> >> Wow! This is awesome. > > Thanks! > >>> 3) Apply Python style fixes. I couldn't help it. This also lets us >>> run flake8 against the Python code automatically, which can find >>> some basic bugs. >> >> Have you thought about creating a make target to run things like >> flake8 to make sure we don't introduce regressions? Are they >> reliable enough that we should run them by default? > > I have indeed, or something close. If you have 'tox' installed, you > just do: > > $ cd python > $ tox -e pep8 > > or this from the root dir of the source tree: > > $ tox -e pep8 -c python/tox.ini > > This runs automatically in 'make check' if tox is installed. There's a > new tox.at that runs it. > > A 'make pep8' target would be a simple one-liner that runs the above > command. configure checks for it, so $HAVE_TOX is also available to > make it conditional. >
I updated the 'make check' integration with a keyword so you can do: $ make check TESTSUITEFLAGS="-k pep8" Another option would be to run this part at ovs build time, similar to a number of other checks currently being done. $ grep 'ALL_LOCAL.*\-check' Makefile.am ALL_LOCAL += config-h-check ALL_LOCAL += printf-check ALL_LOCAL += static-check ALL_LOCAL += thread-safety-check ALL_LOCAL += manpage-check Here's what that would look like. I don't have a strong opinion on whether it belongs just in 'make check' or at build time. > commit bcfb87543435a7c7daba8cc141b1ff38f48b1fb6 > Author: Russell Bryant <russ...@ovn.org> > Date: Sat Dec 19 11:21:54 2015 -0500 > > python: Run pep8 check automatically during build. > > The ovs build process already automatically runs a number of checks on > the code. The tox pep8 environments that runs the flake8 tool is a good > quick sanity check on changes made to Python files, so automatically run > it at build time. It will run once, and then only again if any Python > source files change. > > The first time this runs, it takes about 4.2 seconds on my laptop. > Subsequent runs take about 2.2 seconds. The first time takes a little > longer because tox has to create a Python virtual environment to run the > tests in. > > Signed-off-by: Russell Bryant <russ...@ovn.org> > > diff --git a/.gitignore b/.gitignore > index 71bb21b..19dd88a 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -56,6 +56,7 @@ > /missing > /missing-distfiles > /package.m4 > +/pep8-check > /stamp-h1 > /_build-gcc > /_build-clang > diff --git a/Makefile.am b/Makefile.am > index 966ba27..e80b521 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -338,6 +338,14 @@ manpage-check: $(man_MANS) $(dist_man_MANS) > $(noinst_man_MANS) > CLEANFILES += manpage-check > endif > > +if HAVE_PYTHON > +if HAVE_TOX > +ALL_LOCAL += pep8-check > +pep8-check: $(ovs_pyfiles) $(ovstest_pyfiles) tests/*.py > debian/ovs-monitor-ipsec utilities/ovs-pcap.in vtep/ovs-vtep > ofproto/ipfix-gen-entities > xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync > + if tox -e pep8 -c python/tox.ini; then touch $@; else exit 1; fi > +endif > +endif > + > include $(srcdir)/manpages.mk > $(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.pl > @$(PERL) $(srcdir)/build-aux/sodepends.pl -I. -I$(srcdir) > $(MAN_ROOTS) >$(@F).tmp _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
