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 <agable@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

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)

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.<name>\n'
+          '    tests/git_cl_tests.py TestGitCl.<name>\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']))