Actually, it's filtering by reviews that is useful, clean up the code accordingly

my_reviews.py

Index: my_reviews.py
diff --git a/my_reviews.py b/my_reviews.py
index 4509a489734fcef7a8d39e9e61cdeaa41986669e..22f05a4abae487f2637b27bf81741084bd74014d 100755
--- a/my_reviews.py
+++ b/my_reviews.py
@@ -3,7 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Get rietveld stats.
+"""Get rietveld stats about the review you done, or forgot to do.
 
 Example:
   - my_reviews.py -o me@chromium.org -Q  for stats for last quarter.
@@ -16,43 +16,58 @@ import sys
 import rietveld
 
 
-def print_reviews(owner, reviewer, created_after, created_before, instance_url):
-  """Prints issues with the filter.
+def username(email):
+  return email.split('@', 1)[0]
 
-  Set with_messages=True to search() call bellow if you want each message too.
-  If you only want issue numbers, use keys_only=True in the search() call.
-  You can then use remote.get_issue_properties(issue, True) to get the data per
-  issue.
-  """
+
+def print_reviews(reviewer, created_after, created_before, instance_url):
+  """Prints issues the dude reviewed."""
   remote = rietveld.Rietveld(instance_url, None, None)
+  total = 0
+  actually_reviewed = 0
 
   # See def search() in rietveld.py to see all the filters you can use.
   for issue in remote.search(
-      owner=owner,
       reviewer=reviewer,
       created_after=created_after,
       created_before=created_before,
-      keys_only=False,
-      with_messages=False,
+      with_messages=True,
       ):
+    total += 1
     # By default, hide commit-bot and the domain.
-    reviewers = set(r.split('@', 1)[0] for r in issue['reviewers'])
-    reviewers -= set(('commit-bot',))
+    reviewers = set(username(r) for r in issue['reviewers'])
+    reviewers -= set(['commit-bot'])
     # Strip time.
     timestamp = issue['created'][:10]
+    if any(
+      username(msg['sender']) == username(reviewer)
+      for msg in issue['messages']):
+      reviewed = ' x '
+      actually_reviewed += 1
+    else:
+      reviewed = '   '
 
     # More information is available, print issue.keys() to see them.
-    print '%d: %s %s' % (issue['issue'], timestamp, ', '.join(reviewers))
-
-
-def print_count(owner, reviewer, created_after, created_before, instance_url):
+    print '%7d %s %s O:%-15s  R:%s' % (
+        issue['issue'],
+        timestamp,
+        reviewed,
+        username(issue['owner_email']),
+        ', '.join(reviewers))
+  percent = 0.
+  if total:
+    percent = (actually_reviewed * 100. / total)
+  print 'You actually reviewed %d issues out of %d (%1.1f%%)' % (
+      actually_reviewed, total, percent)
+
+
+def print_count(reviewer, created_after, created_before, instance_url):
   remote = rietveld.Rietveld(instance_url, None, None)
   print len(list(remote.search(
-      owner=owner,
       reviewer=reviewer,
       created_after=created_after,
       created_before=created_before,
-      keys_only=False)))
+      keys_only=True)))
 
 
 def get_previous_quarter(today):
@@ -89,9 +104,9 @@ def main():
       '--count', action='store_true',
       help='Just count instead of printing individual issues')
   parser.add_option(
-      '-o', '--owner', metavar='<email>', help='Filter on issue owner')
-  parser.add_option(
-      '-r', '--reviewer', metavar='<email>', help='Filter on issue reviewer')
+      '-r', '--reviewer', metavar='<email>',
+      default=os.environ.get('EMAIL_ADDRESS'),
+      help='Filter on issue reviewer, default=%default')
   parser.add_option(
       '-c', '--created_after', metavar='<date>',
       help='Filter issues created after the date')
@@ -111,11 +126,8 @@ def main():
   options, args = parser.parse_args()
   if args:
     parser.error('Args unsupported')
-  if not options.owner and not options.reviewer:
-    options.owner = os.environ['EMAIL_ADDRESS']
-    if '@' not in options.owner:
-      parser.error('Please specify at least -o or -r')
-    print >> sys.stderr, 'Defaulting to owner=%s' % options.owner
+
+  print >> sys.stderr, 'Searching for reviews by %s' % options.reviewer
   if options.last_quarter:
     options.created_after = created_after
     options.created_before = created_before
@@ -123,12 +135,12 @@ def main():
         options.created_after, options.created_before)
   if options.count:
     print_count(
-        options.owner, options.reviewer,
+        options.reviewer,
         options.created_after, options.created_before,

[Peter Mayo, 2011/09/22 19:01:05]

can you split the second line of options too, so that this call is
self-consistent w.r.t. linebreaks?

         options.instance_url)
   else:
     print_reviews(
-        options.owner, options.reviewer,
+        options.reviewer,
         options.created_after, options.created_before,

[Peter Mayo, 2011/09/22 19:01:05]

same as above.

         options.instance_url)
   return 0