Merge lp:~rockstar/laika/more-cleanup into lp:laika

[Paul Hummer]

Here's what I did in this branch:

- Moved the global my_bugs var to be a member attr.
- Converted getopt usage to OptionParser, because it's a nicer API, and does the stuff from usage() automatically (try laika.py --help now). This meant I could remove a lot of code, and it'll be much easier to add more args.
- I noticed a bug where user wasn't anything but lp.me, so now we actually specify what user you want.
- There was a now var that was driving me nuts, because it really shouldn't be changed. In python, we don't really have true constants, but we denote variables that shouldn't be changed with all caps. Lame, yes, but I made it obvious.
- Main no longer cared about sys.args, so I killed that.
- cachedir didn't respect $HOME, so if was set to something other that /home/rockstar/, it was broken. I find that with os.path.expanduser, which knows how to expand ~ to $HOME.

The branch itself is bigger than I thought. Sorry about that.

[Alex Chiang (achiang)]

No worries about large changes. I can handle those just fine, as long as the individual commits are sane.

The end result is good, but I'd like to see a little more discipline in the individual commits.

Two examples:

Commit 18 should have been folded into 16 in your development branch, so that I only see the a final, good commit. There's no need to clutter up the development history with bogus commits and then fixup commits. The Linux kernel will never knowingly merge a bad commit and then a fixup commit. [of course, bugs happen, but the goal is certainly clean commit history.] The reason is that bisection is an important tool, and we want to minimize the pain of landing on a bad commit during a bisect run. Of course, we don't have the same problem with laika, but I think it's still worth it to keep this discipline.

The last few lines of commit 14 logically seem like they belong with commit 13. When I was reading 13, I thought to myself, "hm, he added another call to Launchpad.login_with(), but didn't remove the old one... why not?" I then inspected the final diff and saw that we indeed only ended up with a single call to Launchpad.login_with(), but had to re-review the other commits until I found that 14 removed the double call.

Ok, in my opinion, these are nitpicky comments, but they address a real, fundamental philosophy about development.

Thanks for the patches, the refactoring is really nice.

review: Approve

=== modified file 'laika.py'
--- laika.py	2010-07-16 16:23:29 +0000
+++ laika.py	2010-07-17 15:39:40 +0000
@@ -12,20 +12,14 @@
 # GNU General Public License version 2.
 
 import datetime
-import getopt
+from optparse import OptionParser
 import os
 import re
 import sys
 
 from launchpadlib.launchpad import Launchpad
 
-
-# Number of days prior to search for Launchpad activity
-window = 8
-user = os.environ.get("USER")
-
-now = datetime.datetime.utcnow()
-my_bugs = {}
+UTCNOW = datetime.datetime.utcnow()
 
 
 class Report(object):
@@ -40,8 +34,14 @@
     status change.
     '''
 
-    def __init__(self, user):
-        self.user = user
+    bugs = {}
+
+    def __init__(self, user, window):
+        cachedir = "/home/" + user + "/.launchpadlib/cache/"
+        self.launchpad = Launchpad.login_with('laika', 'production', cachedir)
+
+        self.user = self.launchpad.people[user]
+        self.window = window
 
     def print_header(self, header):
         print "==", header, "=="
@@ -52,16 +52,16 @@
         Time zone aware means broken.
         http://www.enricozini.org/2009/debian/using-python-datetime/
         '''
-        win = datetime.timedelta(window)
+        win = datetime.timedelta(self.window)
         date = date.replace(tzinfo=None)
-        delta = now - date
+        delta = UTCNOW - date
         return delta <= win
 
     def print_bugid(self, task):
         '''Using this interface adds the bug to global bug list.'''
         ago = ""
-        my_bugs[task.bug.id] = 1
-        delta = now - task.bug.date_last_updated.replace(tzinfo=None)
+        self.bugs[task.bug.id] = 1
+        delta = UTCNOW - task.bug.date_last_updated.replace(tzinfo=None)
         if delta.days > 0:
             ago = "%d day%s" % (delta.days, "s" if delta.days > 1 else "")
 
@@ -87,7 +87,7 @@
         self.print_header("Assigned Bugs")
 
         for t in tasks:
-            if my_bugs.has_key(t.bug.id):
+            if self.bugs.has_key(t.bug.id):
                 continue
             updates = []
             for s in statuses:
@@ -110,7 +110,7 @@
         tasks = self.user.searchTasks(bug_commenter=self.user)
         self.print_header("Commented Bugs")
         for t in tasks:
-            if my_bugs.has_key(t.bug.id):
+            if self.bugs.has_key(t.bug.id):
                 continue
             for m in t.bug.messages:
                 if m.owner_link != self.user.self_link:
@@ -125,7 +125,7 @@
         tasks = self.user.searchTasks(bug_reporter=self.user)
         self.print_header("Reported Bugs")
         for t in tasks:
-            if my_bugs.has_key(t.bug.id):
+            if self.bugs.has_key(t.bug.id):
                 continue
             if self.in_window(t.bug.date_created):
                 self.print_bugid(t)
@@ -137,45 +137,22 @@
         self.print_reported()
 
 
-def usage():
-    print "Usage:"
-    print "\tlaika [-u <login>] [-w <window>] [-h]"
-    print
-    print "\t-u <login>"
-    print "\t\tSpecify your Launchpad user id."
-    print "\t\tIf not specified, defaults to: %s." % (user)
-    print
-    print "\t-w <num>"
-    print "\t\tLook for activity within the past <num> days."
-    print "\t\tIf not specified, defaults to %d." % (window)
-    print
-    print "\t-h"
-    print "\t\tDisplay this message."
-
-def main(argv):
-
-    try:
-        opts, args = getopt.getopt(argv, "hu:w:")
-    except getopt.GetoptError:
-        usage()
-        sys.exit(2)
-
-    for opt, arg in opts:
-        if opt == '-h':
-            usage()
-            sys.exit()
-        elif opt == '-u':
-            global user
-            user = arg
-        elif opt == '-w':
-            global window
-            window = int(arg)
-
-    cachedir = "/home/" + user + "/.launchpadlib/cache/"
-    lp = Launchpad.login_with('laika', 'production', cachedir)
-
-    report = Report(lp.me)
+def main():
+
+    parser = OptionParser()
+    parser.add_option('-u', '--user', dest='user',
+        default=os.getenv('USER'),
+        help='Specify the Launchpad user id.  '
+             'Defaults to authenticated token owner.')
+    parser.add_option('-w', '--window', dest='window', type='int',
+        default=8,
+        help='Number af days of past activity to look for.  '
+             'Defaults to 8 days')
+
+    options, arguments = parser.parse_args()
+
+    report = Report(options.user, options.window)
     report.render()
 
 if __name__ == "__main__":
-    main(sys.argv[1:])
+    main()