Merge lp:~jcsackett/lp-release-manager-tools/filter-canonical into lp:~sinzui/lp-release-manager-tools/trunk

[j.c.sackett]

Adds date filtering as an option instead of author filtering; provides filtering out commits by Canonical employees within that range.

This branch makes several additions to `report_branch_contributions.py`:
* It adds a date based log formatter, so that commits made within a specified date range can be found.
* It separates finding the matched_revisions from analyzing their changes, for use in juju-reports
* It introduces an additional option to filter out commits made by Canonical employees, so we can find community based commits.

[Curtis Hovey (sinzui)]

Thank you Jon.

I have a couple questions, but I don't see anything blocks your branch.

I prefer datetime.datetime.utcnow() because Lp tends to normalise dates as utc. This wont be true for dates in the revisions, i think. Does utcnow() break your work?

The end_date is not inclusive is it. I need to pass tomorrow's date to get today's date?

review: Needs Information (code)

[j.c.sackett (jcsackett)]

utcnow is fine. I've made that change.

end_date, currently, *is* inclusive (<= end_date). Do we want it to be exclusive?

[lp:~jcsackett/lp-release-manager-tools/filter-canonical]

26. By j.c.sackett on 2014-01-28

Comments from review.

[Curtis Hovey (sinzui)]

Thank you Jon. We do want *inclusive* dates. LGTM.

review: Approve (code)

=== modified file 'report_branch_contributions.py'
--- report_branch_contributions.py	2014-01-15 22:22:38 +0000
+++ report_branch_contributions.py	2014-01-28 17:16:16 +0000
@@ -3,6 +3,7 @@
 
 from __future__ import print_function
 
+import datetime
 from optparse import OptionParser
 import re
 import subprocess
@@ -22,6 +23,43 @@
 DELETES_PATTERN = re.compile(r"(\d+) deletion")
 
 
+class DateLogFormatter(log.LogFormatter):
+    """Log all commits between two dates"""
+
+    supports_merge_revisions = False
+
+    def __init__(self, start_date, end_date, filter_canonical=False):
+        super(DateLogFormatter, self).__init__(to_file=None)
+        self.start_date = start_date
+        self.end_date = end_date
+        self.current_rev = None
+        self.matched_revs = []
+        self.filter = filter_canonical
+
+    def _only_canonical_contributors(self, rev):
+        # Determine if the only authors of the commit are from Canonical
+        canonical = True
+        for author in rev.get_apparent_authors():
+            email = parse_username(author)
+            if '@canonical' not in author:
+                canonical = False
+                break
+        return canonical
+
+    def log_revision(self, lr):
+        """Log a revision that made within the supplied dates.
+
+        :param lr: The LogRevision to check.
+        """
+        if self.filter and self._only_canonical_contributors(lr.rev):
+            # If we're filtering, and this revision has only canonical
+            # contributors, there's no need to check the revision date. 
+            return
+        rev_date = datetime.datetime.fromtimestamp(lr.rev.timestamp)
+        if rev_date >= self.start_date and rev_date <= self.end_date:
+            self.matched_revs.append(lr.rev)
+
+
 class AuthorLogFormatter(log.LogFormatter):
     """Log contributions by an author."""
 
@@ -64,22 +102,24 @@
     return diffstat_proc.communicate()[0].strip()
 
 
-def show_contributions(branch, author, options):
+def get_contributions(branch, log_formatter, start_revno, end_revno=None):
     logger = log.Logger(
         branch, {
             "direction": "forward",
-            "start_revision": options.start_revno,
-            "end_revision": options.end_revno or branch.revno(),
+            "start_revision": start_revno,
+            "end_revision": end_revno or branch.revno(),
             "levels": 0,
         })
-    log_formatter = AuthorLogFormatter(author)
     logger.show(log_formatter)
+    return log_formatter.matched_revs
+
+def show_contributions(branch, matched_revs, options):
     total_commits = 0
     total_inserts = 0
     total_delta = 0
     try:
         branch.lock_read()
-        for rev in log_formatter.matched_revs:
+        for rev in matched_revs:
             total_commits += 1
             revid = rev.revision_id
             diffstat = get_diffstat(
@@ -95,18 +135,49 @@
                 print("Revision %s: %s (delta: %d)" % (revno, diffstat, delta))
     finally:
         branch.unlock()
-
     if options.verbose:
-        print("Total %d commits for %s adding %d lines with a delta of %+d" % (
-            total_commits, author, total_inserts, total_delta))
+        if options.dates:
+            print("Total %d commits from %s to %s adding %d lines with a delta of %+d" % (
+                total_commits, params[0], params[1], total_inserts, total_delta))
+        else:
+            print("Total %d commits for %s adding %d lines with a delta of %+d" % (
+                total_commits, params, total_inserts, total_delta))
     else:
         print("%d:%d:%+d" % (total_commits, total_inserts, total_delta))
 
 
+def get_author(args, branch):
+    if len(args) == 1:
+        author = args[0]
+    else:
+        author = extract_email_address(branch.get_config_stack().get("email"))
+    return author
+
+
+def get_dates(args):
+    fmt = '%Y-%m-%d'
+    if len(args) == 1:
+        start_date = datetime.datetime.strptime(args[0], fmt)
+        end_date = datetime.datetime.utcnow()
+    elif len(args) == 2: 
+        start_date = datetime.datetime.strptime(args[0], fmt)
+        end_date = datetime.datetime.strptime(args[1], fmt)
+    else:
+        start_date = datetime.datetime(1990, 1, 1)
+        end_date = datetime.datetime.utcnow()
+    return (start_date, end_date)
+
+
 def main(args):
-    usage = "%prog [options] [AUTHOR]"
+    usage = "%prog [options] [AUTHOR|START_DATE] [END_DATE]"
     parser = OptionParser(usage=usage)
     parser.add_option(
+        '-d', '--dates', dest='dates', action='store_true', default=False,
+        help='Find revisions by date, not by author.')
+    parser.add_option(
+        '-c', dest='filter_canonical', action='store_true', default=False,
+        help="Filter out revisions contributed only by Canonical developers.")
+    parser.add_option(
         '-s', '--start-revno', dest='start_revno', type='int', default=1900,
         help='Search starting from revno, 1900 is default.')
     parser.add_option(
@@ -119,15 +190,21 @@
         '-v', '--verbose', action='store_true', default=False,
         help='Show detailed information about each revision.')
     options, args = parser.parse_args(args)
-    if len(args) > 1:
+    if ((len(args) > 1 and not options.dates) or
+        (len(args) > 2 and options.dates)):
         parser.print_help()
         return 1
     branch = Branch.open(options.branch)
-    if len(args) == 1:
-        author = args[0]
+    if options.dates:
+        params = get_dates(args)
+        params += (options.filter_canonical,)
+        log_formatter = DateLogFormatter(*params)
     else:
-        author = extract_email_address(branch.get_config_stack().get("email"))
-    show_contributions(branch, author, options)
+        params = get_author(args, branch)
+        log_formatter = AuthorLogFormatter(params)
+    matched_revs = get_contributions(
+        branch, log_formatter, options.start_revno, options.end_revno)
+    show_contributions(branch, matched_revs, options)
     return 0