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

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

The intent for this script is to be like Travis CI for github.

When run, this script will use some of openerp-nag's functions to identify OCA addon MPs.

To these, it will comment on the MP providing information about the and review according to http://pad.openerp.com/p/community-review

Here's a description of the behaviour expected

Gather information about an OpenERP addon MP
* Has this MP been reviewed before by an automated script?
  * Does it contain message identifying as automated?
  * Has there been a new commit since last automated message?
* Does it merge cleanly?
* Is this a new module?
  * Add of a directory and an __openerp__.py file
  * AGPL-3 license check
  * Does it include tests?
  * Does it include demo data?
  * Does it include security files?
  * Does the module version have 2 digits (1.0)
  * Has the .pot file been generated
  * Run flake8 on the entire module
  * Check for deprecated code
  * Check for TODO and FIXME comments
  * Check for debugging breakpoints (ipdb, pdb, set_trace())
  * Test install the module on its own, run tests
  * Does it follow all OpenERP conventions
* Is this a modification of an existing module
  * Which modules are involved?
  * What kind of changes are made?
    * Code behaviour
    * Model changes
      * Has the version been bumped?
      * Any migration scripts included?
    * View changes
    * Tests
      * New tests
      * Added tests
      * Removed tests
    * Demo data
    * Data
    * Translation
    * TODO, FIXME comments
      * Added comments
      * Removed comments
    * Check for debugging breakpoints (ipdb, pdb, set_trace())
  * Evaluate if changes create a net improvement
    * Run flake8 before and after merge
    * Check for deprecated code before and after merge
    * Run tests before and after
      * Are they fixed?
      * Are they broken?
      * Are they unchanged?
      * Which tests results have changed? Are they new/removed?
    * Check conventions before and after merge
* Is this a mixture of both?
  * Separate code of new and modified
* Is this a removal?
* Provide link to http://pad.openerp.com/p/community-review
  * Identify points of concern from the pad
* Vote on MP (or don't)

[lp:~savoirfairelinux-openerp/lp-community-utils/openerp_review]

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

[IMP] added stub functions

32. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-14

Function to list contents of local or remote bzr repository

Given a path, return a list of tracked files in bzr using code for `bzr ls`
Give the list of files without checking out. Useful for scanning launchpad
repos.

Example usgae:
    >>>> lp.list_bzr_repo('lp-community-utils')
    ['lp:lp-community-utils/README.rst',
     'lp:lp-community-utils/checkout-flake8.sh',
     'lp:lp-community-utils/clone_mp_to_community.py',
     'lp:lp-community-utils/merge_mp.py',
     'lp:lp-community-utils/openerp-nag',
     'lp:lp-community-utils/projects',
     'lp:lp-community-utils/replay_missing.py']

May be expanded to support revnos and other flags. The cmd_ls.run() function
signature has the following arguments:
    revision
    verbose
    recursive
    from_root
    unknown
    versioned
    ignored
    null
    kind
    show_ids
    path
    directory
Currently, only path is used.

33. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-16

Check target for openerp addons manifest files

Parse options to target project merge proposals

34. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-18

[IMP] Review project and series implemented

Add logging
Add more options to ls_bzr_repo(), notably the missing recursive option
Separate review_project() into review_series()
Implement skip based on non-addon repo, empty MP repo

35. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-18

Implement been_reviewed() check

Check if there exists a comment older than the last change of the MP which
matches the automated message

36. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-05-07

Implement checking pep8 in review

Unmerged revisions

36. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-05-07

Implement checking pep8 in review

35. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-18

Implement been_reviewed() check

Check if there exists a comment older than the last change of the MP which
matches the automated message

34. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-18

[IMP] Review project and series implemented

Add logging
Add more options to ls_bzr_repo(), notably the missing recursive option
Separate review_project() into review_series()
Implement skip based on non-addon repo, empty MP repo

33. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-16

Check target for openerp addons manifest files

Parse options to target project merge proposals

32. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-14

Function to list contents of local or remote bzr repository

Given a path, return a list of tracked files in bzr using code for `bzr ls`
Give the list of files without checking out. Useful for scanning launchpad
repos.

Example usgae:
    >>>> lp.list_bzr_repo('lp-community-utils')
    ['lp:lp-community-utils/README.rst',
     'lp:lp-community-utils/checkout-flake8.sh',
     'lp:lp-community-utils/clone_mp_to_community.py',
     'lp:lp-community-utils/merge_mp.py',
     'lp:lp-community-utils/openerp-nag',
     'lp:lp-community-utils/projects',
     'lp:lp-community-utils/replay_missing.py']

May be expanded to support revnos and other flags. The cmd_ls.run() function
signature has the following arguments:
    revision
    verbose
    recursive
    from_root
    unknown
    versioned
    ignored
    null
    kind
    show_ids
    path
    directory
Currently, only path is used.

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

[IMP] added stub functions

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

[ADD] stub script for openerp_review along with details in description

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

Moved common code out of openerp-nag into lp.py so it can be used by other scripts

=== modified file 'lp.py'
--- lp.py	2014-05-07 22:28:20 +0000
+++ lp.py	2014-05-07 22:28:20 +0000
@@ -29,6 +29,8 @@
     ProgressBar = None
 
 from launchpadlib.launchpad import Launchpad
+from bzrlib import commands as bzr_commands, builtins as bzr_builtins
+from cStringIO import StringIO
 
 
 # Nag:
@@ -250,3 +252,39 @@
         return Launchpad.login_anonymously(consumer_name, service_root)
     else:
         return Launchpad.login_with(consumer_name, service_root)
+
+
+def list_bzr_repo(repo, recursive=False, *args, **kwargs):
+    """
+    Given a path, return a list of tracked files in bzr using code for `bzr ls`
+    Return the list of files without checking out. Useful for scanning 
+    launchpad repos.
+  
+    Example usage:
+        >>>> lp.list_bzr_repo('lp-community-utils')
+        ['lp:lp-community-utils/README.rst', 
+         'lp:lp-community-utils/checkout-flake8.sh', 
+         'lp:lp-community-utils/clone_mp_to_community.py', 
+         'lp:lp-community-utils/merge_mp.py',
+         'lp:lp-community-utils/openerp-nag', 
+         'lp:lp-community-utils/projects', 
+         'lp:lp-community-utils/replay_missing.py']
+    """
+    bzr_commands.load_plugins()
+    cmd_obj = bzr_builtins.cmd_ls()
+    cmd_obj.outf = StringIO()
+    cmd_obj.run(path=repo, recursive=recursive, *args, **kwargs)
+    return cmd_obj.outf.getvalue().splitlines()
+
+
+def branch_bzr_repo(from_location, to_location, *args, **kwargs):
+    """
+    Given a remote location and a local destination, branch a bzr repo using
+    code for `bzr branch`
+
+    Example usage:
+        >>>> lp.list_bzr_repo('lp-community-utils', '/tmp')
+    """
+    bzr_commands.load_plugins()
+    cmd_obj = bzr_builtins.cmd_branch()
+    cmd_obj.run(from_location, to_location, *args, **kwargs)
 
=== added file 'openerp_review'
--- openerp_review	1970-01-01 00:00:00 +0000
+++ openerp_review	2014-05-07 22:28:20 +0000
@@ -0,0 +1,458 @@
+#!/usr/bin/env python2
+# -*- encoding: utf-8 -*-
+###############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    This module copyright (C) 2010 - 2014 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/>.
+#
+###############################################################################
+
+"""
+Gather information about an OpenERP addon MP
+* Has this MP been reviewed before by an automated script?
+  * Does it contain message identifying as automated?
+  * Has there been a new commit since last automated message?
+* Does it merge cleanly?
+* Is this a new module?
+  * Add of a directory and an __openerp__.py file
+  * AGPL-3 license check
+  * Does it include tests?
+  * Does it include demo data?
+  * Does it include security files?
+  * Does the module version have 2 digits (1.0)
+  * Has the .pot file been generated
+  * Run flake8 on the entire module
+  * Check for deprecated code
+  * Check for TODO and FIXME comments
+  * Check for debugging breakpoints (ipdb, pdb, set_trace())
+  * Test install the module on its own, run tests
+  * Does it follow all OpenERP conventions
+* Is this a modification of an existing module
+  * Which modules are involved?
+  * What kind of changes are made?
+    * Code behaviour
+    * Model changes
+      * Has the version been bumped?
+      * Any migration scripts included?
+    * View changes
+    * Tests
+      * New tests
+      * Added tests
+      * Removed tests
+    * Demo data
+    * Data
+    * Translation
+    * TODO, FIXME comments
+      * Added comments
+      * Removed comments
+    * Check for debugging breakpoints (ipdb, pdb, set_trace())
+  * Evaluate if changes create a net improvement
+    * Run flake8 before and after merge
+    * Check for deprecated code before and after merge
+    * Run tests before and after
+      * Are they fixed?
+      * Are they broken?
+      * Are they unchanged?
+      * Which tests results have changed? Are they new/removed?
+    * Check conventions before and after merge
+* Is this a mixture of both?
+  * Separate code of new and modified
+* Is this a removal?
+* Provide link to http://pad.openerp.com/p/community-review
+  * Identify points of concern from the pad
+* Vote on MP (or don't)
+
+"""
+
+
+from __future__ import (
+    unicode_literals,
+    print_function,
+    absolute_import,
+    division,
+)
+
+import logging
+import os
+import re
+import tempfile
+import shutil
+from pep8 import parse_udiff, StandardReport
+from flake8 import engine
+from argparse import ArgumentParser
+from lp import (
+    launchpad_login,
+    list_bzr_repo,
+    branch_bzr_repo,
+)
+
+consumer_name = u'OpenERP Community Automatic Reviewer Scripts'
+MANIF = u'__openerp__.py'
+AUTOMATED_MESSAGE = u"This is an automated provided by the %s" % consumer_name
+ALL_GOOD_MESSAGE = u"All tests came back clean"
+AUTOMATED_MESSAGE_REGEX = re.compile(AUTOMATED_MESSAGE)
+MESSAGE_CONTAINS_CONFLICTS = u"""\
+Merge proposal contains conflicts on the following files
+  * %s
+Please, rebase or merge with target branch.
+"""
+FLAKE8_IGNORE = u"E501"
+FLAKE8_INIT_IGNORE = u"E501,F401"
+FLAKE8_MANIFEST_IGNORE = u"E501"
+
+
+logging.basicConfig(format=u'%(levelname)s:%(message)s', level=logging.INFO)
+
+
+class NoPrintingPep8Report(StandardReport):
+    def __init__(self, options):
+        super(NoPrintingPep8Report, self).__init__(options=options)
+        self.issues = []
+
+    def get_file_results(self):
+        """Return the result instead of printing it."""
+        self._deferred_print.sort()
+        for line_number, offset, code, text, doc in self._deferred_print:
+            self.issues.append(self._fmt % {
+                'path': self.filename,
+                'row': self.line_offset + line_number, 'col': offset + 1,
+                'code': code, 'text': text,
+            })
+            if self._show_source:
+                if line_number > len(self.lines):
+                    line = ''
+                else:
+                    line = self.lines[line_number - 1]
+                self.issues.append(line.rstrip())
+                self.issues.append(re.sub(r'\S', ' ', line[:offset]) + '^')
+            if self._show_pep8 and doc:
+                self.issues.append('    ' + doc.strip())
+        return self.file_errors
+
+
+def review_series(series, mps):
+    """
+    Given series, conduct review on all open openERP addon MPs
+    Check series if not in format of directory/__openerp__.py or empty
+    Skip MPs which have already been merged
+    """
+    branch = series.branch
+    if not is_openerp_addon_branch(branch):
+        logging.info(u"%s does not seem to be an OpenERP addon repo, "
+                     "skipping..." % branch.bzr_identity)
+        return
+    filtered_mps = filter(lambda m: m.target_branch == branch, mps)
+    if not filtered_mps:
+        logging.info(u"%s does not seem to have any active Merge Proposals, "
+                     "skipping..." % branch.bzr_identity)
+        return
+    for mp in filtered_mps:
+        if been_reviewed(mp):
+            logging.info(
+                u"%s has already been reviewed by this script, "
+                "skipping..." % branch.bzr_identity)
+            return
+        else:
+            issues = review_mp(mp)
+            review = compose_review(mp, issues)
+            post_message_on_launchpad(mp, review)
+            return
+
+
+def review_project(project):
+    """Given repo, conduct review on all series"""
+    mps = project.getMergeProposals()
+    for s in project.series:
+        review_series(s, mps)
+
+
+def is_openerp_addon_branch(branch):
+    """
+    Check if the branch is for OpenERP addons
+    If not, it probably shouldn't be reviewed with this script
+    Repo should either be empty or be of the format module_name/__openerp__.py
+    """
+    files = list_bzr_repo(branch.bzr_identity, recursive=True)
+    files = map(os.path.split, files)
+    manif_files = filter(lambda x: x[1] == MANIF, files)
+    return not files or bool(manif_files)
+
+
+def review_mp(mp):
+    """Perform series of reviews on launchpad merge proposal"""
+    def perform_checks(checks, mp, branch_dir):
+        for check in checks:
+            res = check(mp, branch_dir)
+            if res:
+                yield res
+    checks = [
+        contains_flake8_errors_on_diff,
+    ]
+    conflicts = contains_conflicts(mp)
+    if conflicts:
+        return MESSAGE_CONTAINS_CONFLICTS % '\n  * '.join(conflicts)
+    repo_dir = branch_to_tmp(mp)
+    res = next(perform_checks(checks, mp, repo_dir), ALL_GOOD_MESSAGE)
+    shutil.rmtree(repo_dir)
+    return res
+
+
+def been_reviewed(mp):
+    """
+    Has this MP been reviewed before by an automated script?
+      * Does it contain message identifying as automated?
+      * Has there been a new commit since last automated message?
+    Return boolean of whether or not to skip this MP
+    """
+    last_change = mp.source_branch.date_last_modified
+    return any(c for c in mp.all_comments
+               if c.date_created > last_change
+               and AUTOMATED_MESSAGE_REGEX.match(c.message_body))
+
+
+def branch_to_tmp(mp):
+    """Branch the source branch to a temporary directory"""
+    tmp_dir = os.path.join(tempfile.mkdtemp(prefix='openerp_review'),
+                           mp.source_branch.name)
+    branch_bzr_repo(mp.source_branch.bzr_identity, tmp_dir)
+    return tmp_dir
+
+
+def contains_conflicts(mp):
+    """
+    Does this MP merge cleanly?
+      * If it contains conflicts, suggest rebasing or merging with upstream
+      * Show commands needed to do it
+    Return boolean of whether or not to continue reviewing this MP
+    """
+    return mp.preview_diff.conflicts
+
+
+def contains_flake8_errors_on_diff(mp, branch_dir):
+    """
+    Does this MP introduce pep8 violations?
+
+    Run flake8 against diff
+    Separate __init__.py, __openerp__.py and other files as they have different
+    exceptions.
+    """
+    pushd = os.getcwd()
+    os.chdir(branch_dir)
+    diff = mp.preview_diff.diff_text.open().read()
+    selected_lines = parse_udiff(diff)
+    init_files = []
+    manifest_files = []
+    other_files = []
+    for f in filter(lambda k: k.endswith('.py'), selected_lines.keys()):
+        if os.path.basename(f) == '__init__.py':
+            init_files.append(f)
+        elif os.path.basename(f) == '__openerp__.py':
+            manifest_files.append(f)
+        else:
+            other_files.append(f)
+    init_style = engine.get_style_guide(selected_lines=selected_lines,
+                                        reporter=NoPrintingPep8Report,
+                                        ignore=FLAKE8_INIT_IGNORE,
+                                        quiet=True)
+    init_report = {f: init_style.check_files(f).issues
+                   for f in init_files}
+    manifest_style = engine.get_style_guide(selected_lines=selected_lines,
+                                            reporter=NoPrintingPep8Report,
+                                            ignore=FLAKE8_MANIFEST_IGNORE,
+                                            quiet=True)
+    manifest_report = {f: manifest_style.check_files(f).issues
+                       for f in manifest_files}
+    other_style = engine.get_style_guide(selected_lines=selected_lines,
+                                         reporter=NoPrintingPep8Report,
+                                         ignore=FLAKE8_IGNORE,
+                                         quiet=True)
+    other_report = {f: other_style.check_files(f).issues
+                    for f in other_files}
+    issues = dict(
+        filter(
+            lambda (key, val): val,
+            init_report.items() +
+            manifest_report.items() +
+            other_report.items()
+        )
+    )
+    os.chdir(pushd)
+    return issues
+
+
+def find_modified_modules(mp):
+    """
+    From diff, find which directories modification happened in.
+    Of these directories, those containing __openerp__.py are considered to be
+    modules.
+    Return a list of paths to modules which have had modifications
+    """
+    # TODO
+    raise NotImplementedError(find_modified_modules.__doc__)
+
+
+def get_repo_version(mp):
+    """
+    Identify if this test is being run on a 6.0, 6.1, 7.0, 8.0 or other
+    repo/module.
+    Return version, this should change the behaviour of the tests.
+    """
+    # TODO
+    raise NotImplementedError(get_repo_version.__doc__)
+
+
+def get_modification_type(mp, module_path):
+    """
+    From diff and path, find out if a module has been added, modified, renamed
+    or removed.
+    If a directory and an __openerp__.py file have been added, then it is an
+    added module.
+    """
+    # TODO
+    raise NotImplementedError(get_modification_type.__doc__)
+
+
+def review_new_module(mp, version, module_path):
+    """
+    Review the path according to new_module rules for the version in question.
+    All checks from http://pad.openerp.com/p/community-review should be
+    considered and comments on launchpad should be valid since these are new
+    files. Review should be safe too.
+    * AGPL-3 license check
+    * Does it include tests?
+    * Does it include demo data?
+    * Does it include security files?
+    * Does the module version have 2 digits (1.0)
+    * Has the .pot file been generated
+    * Run flake8 on the entire module
+    * Check for deprecated code
+    * Check for TODO and FIXME comments
+    * Check for debugging breakpoints (ipdb, pdb, set_trace())
+    * Test install the module on its own, run tests
+    * Does it follow all OpenERP conventions
+    """
+    # TODO
+    raise NotImplementedError(review_new_module.__doc__)
+
+
+def identify_changes_to_module(mp, module_path):
+    """
+    Try to breakdown changes made to module to allow review to focus on only
+    that and avoid unrelated comments.
+    * Code behaviour
+    * Model changes
+    * View changes
+    * Tests
+        * New tests
+        * Added tests
+        * Removed tests
+    * Demo data
+    * Data
+    * Translation
+    * TODO, FIXME comments
+        * Added comments
+        * Removed comments
+    * Check for debugging breakpoints (ipdb, pdb, set_trace())
+    """
+    # TODO
+    raise NotImplementedError(identify_changes_to_module.__doc__)
+
+
+def review_modified_module(mp, version, module_path):
+    """
+    See if module passes review process before MP.
+    If it does, do full review, comment on each issue and problem with merge.
+    If it didn't, mention it, then try to calculate net improvement. If changes
+    add or remove problems, mention it. Try to avoid unrelated messages in
+    review (i.e. problems which existed before and have not been touched by MP)
+    * What kind of changes are made?
+    * Evaluate if changes create a net improvement
+        * Run flake8 before and after merge
+        * Perform flake8 on diff generated by git's diff on functions
+            * bzr diff --using git --diff-options "diff -W -p" > diff_file
+            * flake8-python2 --diff diff_file
+        * Check for deprecated code before and after merge
+        * Run tests before and after
+            * Are they fixed?
+            * Are they broken?
+            * Are they unchanged?
+            * Which tests results have changed? Are they new/removed?
+        * Check conventions before and after merge
+    """
+    changes = identify_changes_to_module(mp, module_path)
+    # TODO
+    raise NotImplementedError(review_modified_module.__doc__)
+
+
+def review_renamed_module(mp, version, module_path):
+    """
+    Should do the same review as modified module, but keep in mind that files
+    are in a different place.
+    If there are no changes besides the name of the module path, skip modified
+    module review
+    """
+    # TODO
+    raise NotImplementedError(review_renamed_module.__doc__)
+
+
+def review_removed_module(mp, version, module_path):
+    """
+    Evaluate what was lost, see if any other modules in repo depend on it.
+    If module was non-installable prior, there shouldn't be any issue.
+    """
+    # TODO
+    raise NotImplementedError(review_removed_module.__doc__)
+
+
+def compose_review(mp, issues):
+    """
+    From issues raised by review_* functions, stitch together a nice message to
+    be posted on launchpad
+    """
+    message = AUTOMATED_MESSAGE
+    # TODO
+    logging.warning("TODO: " + compose_review.__doc__)
+    return message
+
+
+def post_message_on_launchpad(mp, message):
+    """Given a message to post on launchpad, post it on the MP."""
+    # TODO
+    logging.warning("TODO: " + post_message_on_launchpad.__doc__)
+
+
+def parse_args():
+    """Define and parse command line arguments for script."""
+    # TODO
+    parser = ArgumentParser()
+    group = parser.add_argument_group()
+    group.add_argument(u'-p', u'--project', metavar="project",
+                       required=True, help=u'Launchpad repository identifier.')
+    return parser.parse_args()
+
+
+def main():
+    """Main Function"""
+    args = parse_args()
+    lp = launchpad_login(False, consumer_name, 'production')
+    project = lp.projects[args.project]
+    review = review_project(project)
+    # TODO
+    raise NotImplementedError(main.__doc__)
+
+if __name__ == "__main__":
+    exit(main())