Merge lp:~savoirfairelinux-openerp/lp-community-utils/branch_pep8 into lp:lp-community-utils

[Sandy Carter (http://www.savoirfairelinux.com)]

Added library for branching and running Flake8/PEP8 checks on changed files in MP

Added run option -t --test-pep8 argument to activate this functionality.

test-pep8 will ignore ocb-addons and ocb-server as it takes too long to branch. This is hard coded, I am willing to move it to argument list if there is demand for it.

[Holger Brunn (Therp) (hbrunn)]

I think it's okay to ignore ocb-addons

review: Approve (code review)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hi,

It checks pep8 on the entire modified file, which is very likely to print way too many errors/warnings: the projects currently are far to be pep8-errors-prone and we don't expect for a merge proposal to fix all the pep8 errors in the modified files. Considering that, I don't see the value to add that in the nag, as it will output a lot of false-negative.

I would agree with something in this vein, that checks only for modified lines (but only git actually): https://github.com/edx/diff-cover

Thanks

review: Disapprove

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

@Guewen, thank you for the link, I was wondering if such a tool existed.

Unmerged revisions

25. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-02-06

Added option to branch MPs and run PEP8 on them

24. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-02-06

Moved nag functionality to library and restructured some code

=== added file 'branch_mp_pep8.py'
--- branch_mp_pep8.py	1970-01-01 00:00:00 +0000
+++ branch_mp_pep8.py	2014-02-06 21:08:54 +0000
@@ -0,0 +1,83 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    This module copyright (C) 2013 Savoir-faire Linux
+#    (<http://www.savoirfairelinux.com>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero 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 Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+import os
+import tempfile
+import shutil
+
+from bzrlib.branch import Branch
+from bzrlib.plugin import load_plugins
+from flake8 import main as flake8
+from flake8 import engine
+
+load_plugins()
+
+
+def branch_pep8(nag):
+    exit_code = 0
+    if nag.project_name in ['ocb-addons', 'ocb-server', 'ocb-web']:
+        print("%s takes too long to branch, skipped..." % nag.project_name)
+        return exit_code
+
+    # Find files changed to test on which to test PEP8
+    changed_files = nag.proposal.preview_diff.diffstat.keys()
+    changed_py_files = sorted([i for i in changed_files if i.endswith(".py")])
+    changed_init_files = [i for i in changed_py_files if i.endswith("__init__.py")]
+    changed_oe_files = [i for i in changed_py_files if i.endswith("__openerp__.py")]
+
+    # Branch MP locally
+    mp_branch_name = nag.proposal.source_branch.bzr_identity
+    mp_branch = Branch.open(mp_branch_name)
+    previousDir = os.getcwd()
+    branch_dir = tempfile.mkdtemp()
+    os.chdir(branch_dir)
+    mp_branch.bzrdir.sprout(branch_dir).open_branch()
+    try:
+        # Create Flake8 style guides
+        flake8_style = engine.get_style_guide(
+            parse_argv=False, config_file=flake8.DEFAULT_CONFIG)
+        flake8_style.options.exclude += ['__init__.py', '__openerp__.py']
+        flake8_style.options.ignore += ('E501', )
+        flake8_style_init = engine.get_style_guide(
+            parse_argv=False, config_file=flake8.DEFAULT_CONFIG)
+        flake8_style_init.options.ignore += ('E501', 'F401', )
+        flake8_style_openerp = engine.get_style_guide(
+            parse_argv=False, config_file=flake8.DEFAULT_CONFIG)
+        flake8_style_openerp.options.ignore += ('E501', )
+
+        # Run Flake8
+        report = flake8_style_init.check_files(paths=changed_init_files)
+        exit_code = exit_code or flake8.print_report(report, flake8_style)
+        report = flake8_style_openerp.check_files(paths=changed_oe_files)
+        exit_code = exit_code or flake8.print_report(report, flake8_style)
+        report = flake8_style.check_files(paths=changed_py_files)
+        exit_code = exit_code or flake8.print_report(report, flake8_style)
+
+    except:
+        pass
+
+    finally:
+        # Cleanup
+        os.chdir(previousDir)
+        shutil.rmtree(branch_dir)
+
+    return exit_code

=== modified file 'openerp-nag'
--- openerp-nag	2014-02-06 21:08:54 +0000
+++ openerp-nag	2014-02-06 21:08:54 +0000
@@ -50,6 +50,7 @@
     gen_project_nags,
     DefaultPolicy,
 )
+from branch_mp_pep8 import branch_pep8
 
 consumer_name = 'OpenERP Community Reviewers Nagging Scripts'
 
@@ -91,6 +92,9 @@
     group.add_argument(
         '--production', action='store_const', const='production',
         dest='service_root', help="Use production launchpad instance")
+    group.add_argument(
+        '-t', '--test-pep8', action='store_true',
+        help="Test branch using Flake8")
     parser.set_defaults(anonymous=True, service_root='production')
     args = parser.parse_args()
     if not args.project and not args.projects_file:
@@ -134,6 +138,8 @@
                   index1=index1, age=nag.age, person=nag.person, 
                   action=nag.action, subject=nag.subject,
                   project=nag.project_name))
+        if args.test_pep8:
+            branch_pep8(nag)
 
 
 if __name__ == "__main__":