Refactoring: Long option names in push and merge scripts.

This CL is split off from https://codereview.chromium.org/173983002/

R=ulan@chromium.org

Review URL: https://codereview.chromium.org/180873010

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19632 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/tools/push-to-trunk/auto_roll.py b/tools/push-to-trunk/auto_roll.py
index 2c71041..e90102d 100755 (executable)
--- a/tools/push-to-trunk/auto_roll.py
+++ b/tools/push-to-trunk/auto_roll.py
@@ -52,9 +52,9 @@ class AutoRollOptions(CommonOptions):
     super(AutoRollOptions, self).__init__(options)
     self.requires_editor = False
     self.status_password = options.status_password
-    self.c = options.c
+    self.chromium = options.chromium
     self.push = getattr(options, 'push', False)
-    self.author = getattr(options, 'a', None)
+    self.author = getattr(options, 'author', None)
 
 
 class Preparation(Step):
@@ -155,7 +155,7 @@ class PushToTrunk(Step):
               push_to_trunk.CONFIG,
               PushToTrunkOptions.MakeForcedOptions(self._options.author,
                                                    self._options.reviewer,
-                                                   self._options.c),
+                                                   self._options.chromium),
               self._side_effect_handler)
       finally:
         self.PushTreeStatus(self["tree_message"])
@@ -181,9 +181,9 @@ def RunAutoRoll(config,
 
 def BuildOptions():
   parser = argparse.ArgumentParser()
-  parser.add_argument("-a", "--author", dest="a",
+  parser.add_argument("-a", "--author",
                       help="The author email used for rietveld.")
-  parser.add_argument("-c", "--chromium", dest="c",
+  parser.add_argument("-c", "--chromium",
                       help=("The path to your Chromium src/ directory to "
                             "automate the V8 roll."))
   parser.add_argument("-p", "--push",
@@ -191,7 +191,7 @@ def BuildOptions():
                       default=False, action="store_true")
   parser.add_argument("-r", "--reviewer",
                       help="The account name to be used for reviews.")
-  parser.add_argument("-s", "--step", dest="s",
+  parser.add_argument("-s", "--step",
                       help="Specify the step where to start work. Default: 0.",
                       default=0, type=int)
   parser.add_argument("--status-password",
@@ -202,7 +202,7 @@ def BuildOptions():
 def Main():
   parser = BuildOptions()
   options = parser.parse_args()
-  if not options.a or not options.c or not options.reviewer:
+  if not options.author or not options.chromium or not options.reviewer:
     print "You need to specify author, chromium src location and reviewer."
     parser.print_help()
     return 1

diff --git a/tools/push-to-trunk/common_includes.py b/tools/push-to-trunk/common_includes.py
index d2f39e7..9ebf3d2 100644 (file)
--- a/tools/push-to-trunk/common_includes.py
+++ b/tools/push-to-trunk/common_includes.py
@@ -230,12 +230,12 @@ class CommonOptions(object):
   def __init__(self, options, manual=True):
     self.requires_editor = True
     self.wait_for_lgtm = True
-    self.s = options.s
+    self.step = options.step
     self.force_readline_defaults = not manual
     self.force_upload = not manual
     self.manual = manual
     self.reviewer = getattr(options, 'reviewer', "")
-    self.author = getattr(options, 'a', "")
+    self.author = getattr(options, 'author', "")
 
 
 class Step(GitRecipesMixin):
@@ -507,7 +507,7 @@ def RunScript(step_classes,
               options,
               side_effect_handler=DEFAULT_SIDE_EFFECT_HANDLER):
   state_file = "%s-state.json" % config[PERSISTFILE_BASENAME]
-  if options.s == 0 and os.path.exists(state_file):
+  if options.step == 0 and os.path.exists(state_file):
     os.remove(state_file)
   state = {}
   steps = []
@@ -515,5 +515,5 @@ def RunScript(step_classes,
     steps.append(MakeStep(step_class, number, state, config,
                           options, side_effect_handler))
 
-  for step in steps[options.s:]:
+  for step in steps[options.step:]:
     step.Run()

diff --git a/tools/push-to-trunk/merge_to_branch.py b/tools/push-to-trunk/merge_to_branch.py
index 7e0e51f..9f7684f 100755 (executable)
--- a/tools/push-to-trunk/merge_to_branch.py
+++ b/tools/push-to-trunk/merge_to_branch.py
@@ -71,7 +71,7 @@ class Preparation(Step):
     if os.path.exists(self.Config(ALREADY_MERGING_SENTINEL_FILE)):
       if self._options.delete_sentinel:
         os.remove(self.Config(ALREADY_MERGING_SENTINEL_FILE))
-      elif self._options.s == 0:
+      elif self._options.step == 0:
         self.Die("A merge is already in progress")
     open(self.Config(ALREADY_MERGING_SENTINEL_FILE), "a").close()
 
@@ -333,7 +333,7 @@ def BuildOptions():
                       default=False, action="store_true")
   parser.add_argument("-p", "--patch", dest="p",
                       help="A patch file to apply as part of the merge.")
-  parser.add_argument("-s", "--step", dest="s",
+  parser.add_argument("-s", "--step",
                       help="The step where to start work. Default: 0.",
                       default=0, type=int)
   return parser

diff --git a/tools/push-to-trunk/push_to_trunk.py b/tools/push-to-trunk/push_to_trunk.py
index ffef74d..d227a50 100755 (executable)
--- a/tools/push-to-trunk/push_to_trunk.py
+++ b/tools/push-to-trunk/push_to_trunk.py
@@ -62,26 +62,26 @@ class PushToTrunkOptions(CommonOptions):
     class Options(object):
       pass
     options = Options()
-    options.s = 0
-    options.l = None
-    options.b = None
-    options.f = True
-    options.m = False
-    options.c = chrome_path
+    options.step = 0
+    options.last_push = None
+    options.last_bleeding_edge = None
+    options.force = True
+    options.manual = False
+    options.chromium = chrome_path
     options.reviewer = reviewer
-    options.a = author
+    options.author = author
     return PushToTrunkOptions(options)
 
   def __init__(self, options):
-    super(PushToTrunkOptions, self).__init__(options, options.m)
-    self.requires_editor = not options.f
-    self.wait_for_lgtm = not options.f
-    self.tbr_commit = not options.m
-    self.l = options.l
+    super(PushToTrunkOptions, self).__init__(options, options.manual)
+    self.requires_editor = not options.force
+    self.wait_for_lgtm = not options.force
+    self.tbr_commit = not options.manual
+    self.last_push = options.last_push
     self.reviewer = options.reviewer
-    self.c = options.c
-    self.b = getattr(options, 'b', None)
-    self.author = getattr(options, 'a', None)
+    self.chromium = options.chromium
+    self.last_bleeding_edge = getattr(options, 'last_bleeding_edge', None)
+    self.author = getattr(options, 'author', None)
 
 
 class Preparation(Step):
@@ -105,7 +105,7 @@ class DetectLastPush(Step):
   MESSAGE = "Detect commit ID of last push to trunk."
 
   def RunStep(self):
-    last_push = self._options.l or self.FindLastTrunkPush()
+    last_push = self._options.last_push or self.FindLastTrunkPush()
     while True:
       # Print assumed commit, circumventing git's pager.
       print self.GitLog(n=1, git_hash=last_push)
@@ -113,10 +113,10 @@ class DetectLastPush(Step):
         break
       last_push = self.FindLastTrunkPush(parent_hash=last_push)
 
-    if self._options.b:
+    if self._options.last_bleeding_edge:
       # Read the bleeding edge revision of the last push from a command-line
       # option.
-      last_push_bleeding_edge = self._options.b
+      last_push_bleeding_edge = self._options.last_bleeding_edge
     else:
       # Retrieve the bleeding edge revision of the last push from the text in
       # the push commit message.
@@ -415,7 +415,7 @@ class CheckChromium(Step):
   MESSAGE = "Ask for chromium checkout."
 
   def Run(self):
-    self["chrome_path"] = self._options.c
+    self["chrome_path"] = self._options.chromium
     if not self["chrome_path"]:
       self.DieNoManualMode("Please specify the path to a Chromium checkout in "
                           "forced mode.")
@@ -547,42 +547,42 @@ def RunPushToTrunk(config,
 def BuildOptions():
   parser = argparse.ArgumentParser()
   group = parser.add_mutually_exclusive_group()
-  group.add_argument("-f", "--force", dest="f",
+  group.add_argument("-f", "--force",
                      help="Don't prompt the user.",
                      default=False, action="store_true")
-  group.add_argument("-m", "--manual", dest="m",
+  group.add_argument("-m", "--manual",
                      help="Prompt the user at every important step.",
                      default=False, action="store_true")
-  parser.add_argument("-a", "--author", dest="a",
+  parser.add_argument("-a", "--author",
                       help="The author email used for rietveld.")
-  parser.add_argument("-b", "--last-bleeding-edge", dest="b",
+  parser.add_argument("-b", "--last-bleeding-edge",
                       help=("The git commit ID of the last bleeding edge "
                             "revision that was pushed to trunk. This is used "
                             "for the auto-generated ChangeLog entry."))
-  parser.add_argument("-c", "--chromium", dest="c",
+  parser.add_argument("-c", "--chromium",
                       help=("The path to your Chromium src/ directory to "
                             "automate the V8 roll."))
-  parser.add_argument("-l", "--last-push", dest="l",
+  parser.add_argument("-l", "--last-push",
                       help="The git commit ID of the last push to trunk.")
   parser.add_argument("-r", "--reviewer",
                       help="The account name to be used for reviews.")
-  parser.add_argument("-s", "--step", dest="s",
+  parser.add_argument("-s", "--step",
                       help="The step where to start work. Default: 0.",
                       default=0, type=int)
   return parser
 
 
 def ProcessOptions(options):
-  if options.s < 0:
-    print "Bad step number %d" % options.s
+  if options.step < 0:
+    print "Bad step number %d" % options.step
     return False
-  if not options.m and not options.reviewer:
+  if not options.manual and not options.reviewer:
     print "A reviewer (-r) is required in (semi-)automatic mode."
     return False
-  if not options.m and not options.c:
+  if not options.manual and not options.chromium:
     print "A chromium checkout (-c) is required in (semi-)automatic mode."
     return False
-  if not options.m and not options.a:
+  if not options.manual and not options.author:
     print "Specify your chromium.org email with -a in (semi-)automatic mode."
     return False
   return True

diff --git a/tools/push-to-trunk/test_scripts.py b/tools/push-to-trunk/test_scripts.py
index 7a6df7e..657a086 100644 (file)
--- a/tools/push-to-trunk/test_scripts.py
+++ b/tools/push-to-trunk/test_scripts.py
@@ -78,14 +78,14 @@ def MakeOptions(s=0, l=None, f=False, m=True, r=None, c=None, a=None,
   class Options(object):
       pass
   options = Options()
-  options.s = s
-  options.l = l
-  options.f = f
-  options.m = m
+  options.step = s
+  options.last_push = l
+  options.force = f
+  options.manual = m
   options.reviewer = r
-  options.c = c
-  options.a = a
-  options.p = p
+  options.chromium = c
+  options.author = a
+  options.push = p
   options.status_password = status_password
   options.revert_bleeding_edge = revert_bleeding_edge
   return options
@@ -992,7 +992,7 @@ LOG=N
                                  self))
 
     # Test that state recovery after restarting the script works.
-    options.s = 3
+    options.step = 3
     RunMergeToBranch(TEST_CONFIG, MergeToBranchOptions(options), self)