Use CLs more consistently instead of branch names

git_cl.py

Index: git_cl.py
diff --git a/git_cl.py b/git_cl.py
index cf1e2c91093d9d73a21982b9336e1ce757b6a7ed..2927fd6622c22edb2c01704aed2b752c08170329 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -17,11 +17,9 @@ import json
 import logging
 import optparse
 import os
-import Queue
 import re
 import stat
 import sys
-import tempfile
 import textwrap
 import time
 import traceback
@@ -2896,9 +2894,8 @@ def color_for_status(status):
     'error': Fore.WHITE,
   }.get(status, Fore.WHITE)
 
-def fetch_cl_status(branch, auth_config=None):
+def fetch_cl_status(cl, auth_config=None):

[tandrii(chromium), 2016/04/15 15:38:35]

auth_config isn't needed any more.

   """Fetches information for an issue and returns (branch, issue, status)."""
-  cl = Changelist(branchref=branch, auth_config=auth_config)
   url = cl.GetIssueURL()
   status = cl.GetStatus()
 
@@ -2906,10 +2903,10 @@ def fetch_cl_status(branch, auth_config=None):
     # The issue probably doesn't exist anymore.
     url += ' (broken)'

[tandrii(chromium), 2016/04/15 15:38:35]

i think with your awesome refactoring, we can go further and remove this
function.

cl.GetIssueUrl() doesn't block, and can be computed on the fly in status_str
(line 3117). The logic for " (broken)" can be moved there too.

then the only remaining bit here is just calling cl.GetStatus(), which can be
used directly in line 2926.

[Clemens Hammacher, 2016/04/21 12:54:46]

Removing this function would conflict with this other CL:
https://codereview.chromium.org/1895463002
I will address the other comments though.

 
-  return (branch, url, status)
+  return (cl, url, status)
 
 def get_cl_statuses(
-    branches, fine_grained, max_processes=None, auth_config=None):
+    changes, fine_grained, max_processes=None, auth_config=None):
   """Returns a blocking iterable of (branch, issue, color) for given branches.
 
   If fine_grained is true, this will fetch CL statuses from the server.
@@ -2925,23 +2922,23 @@ def get_cl_statuses(
   if fine_grained:
     # Process one branch synchronously to work through authentication, then
     # spawn processes to process all the other branches in parallel.
-    if branches:
-      fetch = lambda branch: fetch_cl_status(branch, auth_config=auth_config)
-      yield fetch(branches[0])
+    if changes:
+      fetch = lambda cl: fetch_cl_status(cl, auth_config=auth_config)

[tandrii(chromium), 2016/04/15 15:38:35]

so based on above comment, i propose to change this to:

fetch = lambda cl: (cl, cl.GetStatus())

+      yield fetch(changes[0])
 
-      branches_to_fetch = branches[1:]
+      changes_to_fetch = changes[1:]
       pool = ThreadPool(
-          min(max_processes, len(branches_to_fetch))
+          min(max_processes, len(changes_to_fetch))
               if max_processes is not None
-              else len(branches_to_fetch))
-      for x in pool.imap_unordered(fetch, branches_to_fetch):
+              else len(changes_to_fetch))
+      for x in pool.imap_unordered(fetch, changes_to_fetch):
         yield x
   else:
     # Do not use GetApprovingReviewers(), since it requires an HTTP request.
-    for b in branches:
-      cl = Changelist(branchref=b, auth_config=auth_config)
+    for c in changes:
+      cl = Changelist(branchref=c.GetBranch(), auth_config=auth_config)

[tandrii(chromium), 2016/04/15 15:38:35]

I think you can just use one from changes list, and if so, nit: s/c/cl

       url = cl.GetIssueURL()
-      yield (b, url, 'waiting' if url else 'error')
+      yield (c, url, 'waiting' if url else 'error')

[tandrii(chromium), 2016/04/15 15:38:35]

and based on refactoring suggestion for line 2926, this can become just:

yield (cl, 'waiting' if cl.GetIssueURL() else 'error')

 
 
 def upload_branch_deps(cl, args):
@@ -3094,24 +3091,22 @@ def CMDstatus(parser, args):
     print('No local branch found.')
     return 0
 
-  changes = (
+  changes = [
       Changelist(branchref=b, auth_config=auth_config)
-      for b in branches.splitlines())
-  # TODO(tandrii): refactor to use CLs list instead of branches list.
-  branches = [c.GetBranch() for c in changes]
-  alignment = max(5, max(len(b) for b in branches))
+      for b in branches.splitlines()]
   print 'Branches associated with reviews:'
-  output = get_cl_statuses(branches,
+  output = get_cl_statuses(changes,
                            fine_grained=not options.fast,
                            max_processes=options.maxjobs,
                            auth_config=auth_config)
 
   branch_statuses = {}
-  alignment = max(5, max(len(ShortBranchName(b)) for b in branches))
-  for branch in sorted(branches):
+  alignment = max(5, max(len(ShortBranchName(c.GetBranch())) for c in changes))
+  for cl in sorted(changes, key = lambda c : c.GetBranch()):

[tandrii(chromium), 2016/04/15 15:38:35]

nits: no space before ":" and no space around "=" for key-value arguments.

=>
for cl in sorted(changes, key=lambda c: c.GetBranch()):

+    branch = cl.GetBranch()
     while branch not in branch_statuses:
-      b, i, status = output.next()
-      branch_statuses[b] = (i, status)
+      c, i, status = output.next()
+      branch_statuses[c.GetBranch()] = (i, status)
     issue_url, status = branch_statuses.pop(branch)
     color = color_for_status(status)
     reset = Fore.RESET
@@ -3120,8 +3115,8 @@ def CMDstatus(parser, args):
       reset = ''
     status_str = '(%s)' % status if status else ''
     print '  %*s : %s%s %s%s' % (
-          alignment, ShortBranchName(branch), color, issue_url, status_str,
-          reset)
+          alignment, ShortBranchName(branch), color, issue_url,
+          status_str, reset)
 
   cl = Changelist(auth_config=auth_config)
   print