Merge lp:~elopio/unity8/flake8-precommit into lp:unity8

[Leo Arias]

Added a precommit hook for static tests.

 * Are there any related MPs required for this MP to build/function as expected? Please list.

The prerequisite branch, https://code.launchpad.net/~elopio/unity8/flake8

 * Did you perform an exploratory manual test run of your code change and any related functionality?

Yes. Tried with the hook, without the hook, with the run script, without the run script, with errors and without errors.

 * Did you make sure that your branch does not contain spurious tags?

Yes.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

No packaging changes.

 * If you changed the UI, has there been a design review?

No UI changes.

[lp:~elopio/unity8/flake8-precommit]

981. By Leo Arias on 2014-06-27

Cleaned the call.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:981
http://jenkins.qa.ubuntu.com/job/unity8-ci/3256/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1308
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/260
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/350
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/350
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/350/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/350
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1624
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2211
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2211/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8963

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3256/rebuild

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

Hmm we actually wanted a common "pre_push.sh" or so, that would run:
- cmake --build builddir --target test
- flake
- generate and check if qmltypes are updated

Ideally (but tricky without comparing to trunk, so maybe optional...)
- check whether any pngs are being added/modified and remind to use optipng on them

So, please rename the script to be generic, add the cmake check to it, and drop the original hook.

review: Needs Fixing

[Michael Zanetti (mzanetti)]

Do we really need 2 hooks?

I guess it can be a single bzr hook that calls multiple scripts/commands.

afaics the pyflake hook would prevent me from committing if something is not ok. For the other hook we intentionally only print a warning to the user and allow to uncommit, fix, recommit (reusing the commit message) or ignore the hook if pushing to a temporary branch. I'd prefer this one to behave the same, basically just including the python checks (and whatever Michal listed above) in the existing hook.

review: Needs Information

[Michał Sawicz (saviq)]

Yes agreed, the hook should warn, not block.

Unmerged revisions

981. By Leo Arias on 2014-06-27

Cleaned the call.

980. By Leo Arias on 2014-06-27

Improved the hook, added a check for mccabe complexity.

979. By Leo Arias on 2014-06-27

Added the static tests pre-commit hook.

=== modified file '.bazaar/Makefile'
--- .bazaar/Makefile	2013-06-05 22:03:08 +0000
+++ .bazaar/Makefile	2014-06-27 15:11:16 +0000
@@ -4,3 +4,4 @@
 
 uninstall:
 	rm ~/.bazaar/plugins/makecheck_unity_phablet.py*
+	rm ~/.bazaar/plugins/run_static_tests.py*

=== added file '.bazaar/plugins/run_static_tests.py'
--- .bazaar/plugins/run_static_tests.py	1970-01-01 00:00:00 +0000
+++ .bazaar/plugins/run_static_tests.py	2014-06-27 15:11:16 +0000
@@ -0,0 +1,47 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+#
+# Unity Static Pre-commit Tests
+# Copyright (C) 2014 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import subprocess
+
+from bzrlib import branch, errors
+
+
+def run_static_tests(
+        local_branch, master_branch, old_revision_number, old_revision_id,
+        future_revision_number, future_revision_id, tree_delta, future_tree):
+    """Run static tests on the source code."""
+    _run_python_static_tests()
+
+
+def _run_python_static_tests():
+    """Run static tests on python code."""
+    if os.path.exists('run_python_static_tests.sh'):
+        print('Running the python static tests.')
+        try:
+            subprocess.check_call(
+                os.path.abspath('run_python_static_tests.sh'))
+        except subprocess.CalledProcessError:
+            raise errors.BzrError('Python static tests failed.')
+    else:
+        print('The python static tests will not be run.')
+
+
+branch.Branch.hooks.install_named_hook(
+    'pre_commit', run_static_tests, 'Run static tests before commit')

=== added file 'run_python_static_tests.sh'
--- run_python_static_tests.sh	1970-01-01 00:00:00 +0000
+++ run_python_static_tests.sh	2014-06-27 15:11:16 +0000
@@ -0,0 +1,5 @@
+#!/bin/sh
+
+# Flake8 checks code style using pep8, static erros using pyflakes and code
+# complexity using mccabe.
+flake8 . --max-complexity 10