From 3a16ed155e3f7ac56db71f207d5779fc97c3bae8 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Thu, 23 Mar 2017 10:51:55 -0700 Subject: [PATCH] Use Bug: footer for Gerrit CLs Note that because it is now a gerrit footer, it both appears in the same block as the Change-Id footer (no blank line between them), and isn't guaranteed to be above the Change-Id footer. This doesn't matter during "git cl upload", when a Change-Id hasn't been allocated yet, but will show up during "git cl description". Bug: 681184 Change-Id: I2ab6fc13be8e992709618a666012410b1a7c02de Reviewed-on: https://chromium-review.googlesource.com/446660 Commit-Queue: Aaron Gable Reviewed-by: Robbie Iannucci --- git_cl.py | 30 +++++++++--------------------- tests/git_cl_test.py | 15 ++++++--------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/git_cl.py b/git_cl.py index f65b60da0..f001974c7 100755 --- a/git_cl.py +++ b/git_cl.py @@ -838,19 +838,6 @@ class Settings(object): self.viewvc_url = self._GetRietveldConfig('viewvc-url', error_ok=True) return self.viewvc_url - def GetBugLineFormat(self): - # rietveld.bug-line-format should have a %s where the list of bugs should - # go. This is a bit of a quirk, because normal people will always want the - # bug list to go right after a prefix like BUG= or Bug:. The %s format - # approach is used strictly because there isn't a great way to carry the - # desired space after Bug: all the way from codereview.settings to here - # without treating : specially or inventing a quoting scheme. - bug_line_format = self._GetRietveldConfig('bug-line-format', error_ok=True) - if not bug_line_format: - # TODO(tandrii): change this to 'Bug: %s' to be a proper Gerrit footer. - bug_line_format = 'BUG=%s' - return bug_line_format - def GetBugPrefix(self): return self._GetRietveldConfig('bug-prefix', error_ok=True) @@ -2182,7 +2169,7 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase): options.tbr_owners, change) if not options.force: - change_desc.prompt(bug=options.bug) + change_desc.prompt(bug=options.bug, git_footer=False) if not change_desc.description: print('Description is empty; aborting.') @@ -3133,7 +3120,7 @@ class ChangeDescription(object): """Contains a parsed form of the change description.""" R_LINE = r'^[ \t]*(TBR|R)[ \t]*=[ \t]*(.*?)[ \t]*$' CC_LINE = r'^[ \t]*(CC)[ \t]*=[ \t]*(.*?)[ \t]*$' - BUG_LINE = r'^[ \t]*(BUGS?|Bugs?)[ \t]*[:=][ \t]*(.*?)[ \t]*$' + BUG_LINE = r'^[ \t]*(?:(BUG)[ \t]*=|Bug:)[ \t]*(.*?)[ \t]*$' CHERRY_PICK_LINE = r'^\(cherry picked from commit [a-fA-F0-9]{40}\)$' def __init__(self, description): @@ -3206,7 +3193,7 @@ class ChangeDescription(object): if new_tbr_line: self.append_footer(new_tbr_line) - def prompt(self, bug=None): + def prompt(self, bug=None, git_footer=True): """Asks the user to update the description.""" self.set_description([ '# Enter a description of the change.', @@ -3220,9 +3207,11 @@ class ChangeDescription(object): if not any((regexp.match(line) for line in self._description_lines)): prefix = settings.GetBugPrefix() values = list(_get_bug_line_values(prefix, bug or '')) or [prefix] - bug_line_format = settings.GetBugLineFormat() - for value in values: - self.append_footer(bug_line_format % value) + if git_footer: + self.append_footer('Bug: %s' % ', '.join(values)) + else: + for value in values: + self.append_footer('BUG=%s' % value) content = gclient_utils.RunEditor(self.description, True, git_editor=settings.GetGitEditor()) @@ -3394,7 +3383,6 @@ def LoadCodereviewSettingsFromFile(fileobj): SetProperty('private', 'PRIVATE', unset_error_ok=True) SetProperty('tree-status-url', 'STATUS', unset_error_ok=True) SetProperty('viewvc-url', 'VIEW_VC', unset_error_ok=True) - SetProperty('bug-line-format', 'BUG_LINE_FORMAT', unset_error_ok=True) SetProperty('bug-prefix', 'BUG_PREFIX', unset_error_ok=True) SetProperty('cpplint-regex', 'LINT_REGEX', unset_error_ok=True) SetProperty('cpplint-ignore-regex', 'LINT_IGNORE_REGEX', unset_error_ok=True) @@ -4372,7 +4360,7 @@ def CMDdescription(parser, args): description.set_description(text) else: - description.prompt() + description.prompt(git_footer=cl.IsGerrit()) if cl.GetDescription().strip() != description.description: cl.UpdateDescription(description.description, force=options.force) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 861d3692d..7b96d95e8 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -568,14 +568,14 @@ class TestGitCl(TestCase): '\nIF YOU SEE THIS, READ BELOW, IT WILL SAVE YOUR TIME!\n' 'There are un-consumed self.calls after this test has finished.\n' 'If you don\'t know which test this is, run:\n' - ' tests/git_cl_test.py -v\n' + ' tests/git_cl_tests.py -v\n' '\n' 'If you are already running just this single test, then **first** ' 'fix the problem whose exception is emitted below by unittest ' 'runner.\n' '\n' 'Else, to be sure what\'s going on, run this test **alone** with \n' - ' tests/git_cl_test.py TestGitCl.\n' + ' tests/git_cl_tests.py TestGitCl.\n' 'and follow instructions above.\n' + '=' * 80) finally: @@ -638,7 +638,6 @@ class TestGitCl(TestCase): ((['git', 'config', '--unset-all', 'rietveld.private'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.tree-status-url'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.viewvc-url'],), CERR1), - ((['git', 'config', '--unset-all', 'rietveld.bug-line-format'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.bug-prefix'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.cpplint-regex'],), CERR1), ((['git', 'config', '--unset-all', 'rietveld.cpplint-ignore-regex'],), @@ -722,8 +721,7 @@ class TestGitCl(TestCase): ((['git', 'log', '--pretty=format:%s\n\n%b', 'fake_ancestor_sha..HEAD'],), 'desc\n'), - ((['git', 'config', 'rietveld.bug-prefix'],), CERR1), - ((['git', 'config', 'rietveld.bug-line-format'],), CERR1), + ((['git', 'config', 'rietveld.bug-prefix'],), ''), ] @classmethod @@ -2250,13 +2248,13 @@ class TestGitCl(TestCase): '# The first line will also be used as the subject of the review.\n' '#--------------------This line is 72 characters long' '--------------------\n' - 'Some.\n\nBUG=\n\nChange-Id: xxx', + 'Some.\n\nChange-Id: xxx\nBug: ', desc) # Simulate user changing something. - return 'Some.\n\nBUG=123\n\nChange-Id: xxx' + return 'Some.\n\nChange-Id: xxx\nBug: 123' def UpdateDescriptionRemote(_, desc, force=False): - self.assertEquals(desc, 'Some.\n\nBUG=123\n\nChange-Id: xxx') + self.assertEquals(desc, 'Some.\n\nChange-Id: xxx\nBug: 123') self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.mock(git_cl.Changelist, 'GetDescription', @@ -2270,7 +2268,6 @@ class TestGitCl(TestCase): ((['git', 'config', 'branch.feature.gerritissue'],), '123'), ((['git', 'config', 'rietveld.autoupdate'],), CERR1), ((['git', 'config', 'rietveld.bug-prefix'],), CERR1), - ((['git', 'config', 'rietveld.bug-line-format'],), CERR1), ((['git', 'config', 'core.editor'],), 'vi'), ] self.assertEqual(0, git_cl.main(['description', '--gerrit']))