From 9abde8c3f3a2c0cfb2b78f772655352e094e612e Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Tue, 17 Nov 2020 08:44:52 +0000 Subject: [PATCH] Reland "Add option to git cl st to sort by committer date" This is a reland of 9ca89ac1f4b0101de2c774c9d98bd50917d8b5b8 Original change's description: > Add option to git cl st to sort by committer date > > Some developers want to see the branches with recent commits first. > This CL adds the option --dateorder to git cl st which sorts branches > by the committer date of the most recent branch. > > Change-Id: Ibdf3fc35ea784521a3e99b374535f822bb83a55e > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2521573 > Commit-Queue: Sigurd Schneider > Reviewed-by: Josip Sokcevic > Reviewed-by: Dirk Pranke Change-Id: I974ac297fcf8652855b101f58a98b3cd122fce36 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2532835 Commit-Queue: Sigurd Schneider Reviewed-by: Josip Sokcevic Reviewed-by: Dirk Pranke --- git_cl.py | 34 +++++++++++++++++--- tests/git_cl_test.py | 75 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/git_cl.py b/git_cl.py index d2fb3faa2..f1772c7ee 100755 --- a/git_cl.py +++ b/git_cl.py @@ -939,7 +939,11 @@ class Changelist(object): with great care. """ - def __init__(self, branchref=None, issue=None, codereview_host=None): + def __init__(self, + branchref=None, + issue=None, + codereview_host=None, + commit_date=None): """Create a new ChangeList instance. **kwargs will be passed directly to Gerrit implementation. @@ -956,6 +960,7 @@ class Changelist(object): self.branch = scm.GIT.ShortBranchName(self.branchref) else: self.branch = None + self.commit_date = commit_date self.upstream_branch = None self.lookedup_issue = False self.issue = issue or None @@ -994,6 +999,10 @@ class Changelist(object): """Extends the list of users to cc on this CL based on the changed files.""" self.more_cc.extend(more_cc) + def GetCommitDate(self): + """Returns the commit date as provided in the constructor""" + return self.commit_date + def GetBranch(self): """Returns the short branch name, e.g. 'main'.""" if not self.branch: @@ -2680,6 +2689,7 @@ class ChangeDescription(object): fixed_regexp = re.compile(self.FIXED_LINE) prefix = settings.GetBugPrefix() has_issue = lambda l: bug_regexp.match(l) or fixed_regexp.match(l) + if not any((has_issue(line) for line in self._description_lines)): self.append_footer('Bug: %s' % prefix) @@ -3495,6 +3505,10 @@ def CMDstatus(parser, args): '-i', '--issue', type=int, help='Operate on this issue instead of the current branch\'s implicit ' 'issue. Requires --field to be set.') + parser.add_option('-d', + '--date-order', + action='store_true', + help='Order branches by committer date.') options, args = parser.parse_args(args) if args: parser.error('Unsupported args: %s' % args) @@ -3523,14 +3537,17 @@ def CMDstatus(parser, args): print(url) return 0 - branches = RunGit(['for-each-ref', '--format=%(refname)', 'refs/heads']) + branches = RunGit([ + 'for-each-ref', '--format=%(refname) %(committerdate:unix)', 'refs/heads' + ]) if not branches: print('No local branch found.') return 0 changes = [ - Changelist(branchref=b) - for b in branches.splitlines()] + Changelist(branchref=b, commit_date=ct) + for b, ct in map(lambda line: line.split(' '), branches.splitlines()) + ] print('Branches associated with reviews:') output = get_cl_statuses(changes, fine_grained=not options.fast, @@ -3556,7 +3573,13 @@ def CMDstatus(parser, args): branch_statuses = {} alignment = max(5, max(len(FormatBranchName(c.GetBranch())) for c in changes)) - for cl in sorted(changes, key=lambda c: c.GetBranch()): + if options.date_order: + sorted_changes = sorted(changes, + key=lambda c: c.GetCommitDate(), + reverse=True) + else: + sorted_changes = sorted(changes, key=lambda c: c.GetBranch()) + for cl in sorted_changes: branch = cl.GetBranch() while branch not in branch_statuses: c, status = next(output) @@ -5197,6 +5220,7 @@ def CMDlol(parser, args): class OptionParser(optparse.OptionParser): """Creates the option parse and add --verbose support.""" + def __init__(self, *args, **kwargs): optparse.OptionParser.__init__( self, *args, prog='git cl', version=__version__, **kwargs) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index a4a0cbc20..18852b1c8 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2951,7 +2951,7 @@ class CMDTestCaseBase(unittest.TestCase): 'current_revision': 'beeeeeef', 'revisions': { 'deadbeaf': { - '_number': 6, + '_number': 6, 'kind': 'REWORK', }, 'beeeeeef': { @@ -3810,6 +3810,79 @@ class CMDFormatTestCase(unittest.TestCase): self._check_yapf_filtering(files, expected) +class CMDStatusTestCase(CMDTestCaseBase): + # Return branch names a,..,f with comitterdates in increasing order, i.e. + # 'f' is the most-recently changed branch. + def _mock_run_git(commands): + if commands == [ + 'for-each-ref', '--format=%(refname) %(committerdate:unix)', + 'refs/heads' + ]: + branches_and_committerdates = [ + 'refs/heads/a 1', + 'refs/heads/b 2', + 'refs/heads/c 3', + 'refs/heads/d 4', + 'refs/heads/e 5', + 'refs/heads/f 6', + ] + return '\n'.join(branches_and_committerdates) + + # Mock the status in such a way that the issue number gives us an + # indication of the commit date (simplifies manual debugging). + def _mock_get_cl_statuses(branches, fine_grained, max_processes): + for c in branches: + c.issue = (100 + int(c.GetCommitDate())) + yield (c, 'open') + + @mock.patch('git_cl.Changelist.EnsureAuthenticated') + @mock.patch('git_cl.Changelist.FetchDescription', lambda cl, pretty: 'x') + @mock.patch('git_cl.Changelist.GetIssue', lambda cl: cl.issue) + @mock.patch('git_cl.RunGit', _mock_run_git) + @mock.patch('git_cl.get_cl_statuses', _mock_get_cl_statuses) + @mock.patch('git_cl.Settings.GetRoot', return_value='') + @mock.patch('scm.GIT.GetBranch', return_value='a') + def testStatus(self, *_mocks): + self.assertEqual(0, git_cl.main(['status', '--no-branch-color'])) + self.maxDiff = None + self.assertEqual( + sys.stdout.getvalue(), 'Branches associated with reviews:\n' + ' * a : https://crrev.com/c/101 (open)\n' + ' b : https://crrev.com/c/102 (open)\n' + ' c : https://crrev.com/c/103 (open)\n' + ' d : https://crrev.com/c/104 (open)\n' + ' e : https://crrev.com/c/105 (open)\n' + ' f : https://crrev.com/c/106 (open)\n\n' + 'Current branch: a\n' + 'Issue number: 101 (https://chromium-review.googlesource.com/101)\n' + 'Issue description:\n' + 'x\n') + + @mock.patch('git_cl.Changelist.EnsureAuthenticated') + @mock.patch('git_cl.Changelist.FetchDescription', lambda cl, pretty: 'x') + @mock.patch('git_cl.Changelist.GetIssue', lambda cl: cl.issue) + @mock.patch('git_cl.RunGit', _mock_run_git) + @mock.patch('git_cl.get_cl_statuses', _mock_get_cl_statuses) + @mock.patch('git_cl.Settings.GetRoot', return_value='') + @mock.patch('scm.GIT.GetBranch', return_value='a') + def testStatusByDate(self, *_mocks): + self.assertEqual( + 0, git_cl.main(['status', '--no-branch-color', '--date-order'])) + self.maxDiff = None + self.assertEqual( + sys.stdout.getvalue(), 'Branches associated with reviews:\n' + ' f : https://crrev.com/c/106 (open)\n' + ' e : https://crrev.com/c/105 (open)\n' + ' d : https://crrev.com/c/104 (open)\n' + ' c : https://crrev.com/c/103 (open)\n' + ' b : https://crrev.com/c/102 (open)\n' + ' * a : https://crrev.com/c/101 (open)\n\n' + 'Current branch: a\n' + 'Issue number: 101 (https://chromium-review.googlesource.com/101)\n' + 'Issue description:\n' + 'x\n') + + if __name__ == '__main__': logging.basicConfig( level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)