bot_update: understand new Gerrit patch properties.

recipe_modules/bot_update/api.py

Index: recipe_modules/bot_update/api.py
diff --git a/recipe_modules/bot_update/api.py b/recipe_modules/bot_update/api.py
index be231c85d42e3023003d4768fe64958b11b65c91..2b179cbca67f5c3bf39ed3a85cf8669bcec153e6 100644
--- a/recipe_modules/bot_update/api.py
+++ b/recipe_modules/bot_update/api.py
@@ -10,13 +10,15 @@ from recipe_engine import recipe_api
 
 class BotUpdateApi(recipe_api.RecipeApi):
 
-  def __init__(self, issue, patchset, repository, gerrit_ref, rietveld,
-               revision, parent_got_revision, deps_revision_overrides,
-               fail_patch, *args, **kwargs):
-    self._issue = issue
-    self._patchset = patchset
-    self._repository = repository
-    self._gerrit_ref = gerrit_ref
+  def __init__(self, issue, patch_issue, patchset, patch_set, patch_project,

[Michael Achenbach, 2016/10/24 08:18:13]

Sure this isn't called anywhere with args? Maybe to be safe, put new keywords in
the end with defaults? And land a new sorting as secondary refactoring CL?
(unless you're sure)

[tandrii(chromium), 2016/10/24 11:43:37]

I wasn't just like you, but I am now after reading the recipe_engine source.

+               repository, patch_repository_url, gerrit_ref, patch_ref,
+               patch_gerrit_url, rietveld, revision, parent_got_revision,
+               deps_revision_overrides, fail_patch, *args, **kwargs):
+    self._issue = issue or patch_issue
+    self._patchset = patchset or patch_set

[Michael Achenbach, 2016/10/24 08:18:13]

Are these strings or integers? I assume 0 can't ever be a valid number, right?

[tandrii(chromium), 2016/10/24 11:43:37]

strings for now.

+    self._repository = repository or patch_repository_url
+    self._gerrit_ref = gerrit_ref or patch_ref
+    self._gerrit = patch_gerrit_url
     self._rietveld = rietveld
     self._revision = revision
     self._parent_got_revision = parent_got_revision
@@ -124,6 +126,12 @@ class BotUpdateApi(recipe_api.RecipeApi):
     if not gerrit_ref or not gerrit_repo:
       gerrit_repo = gerrit_ref = None
     assert (gerrit_ref != None) == (gerrit_repo != None)
+    if gerrit_ref:
+      # Gerrit patches have historically not specified issue and patchset.
+      # resourece/bot_update has as a result implicit assumption that set issue
+      # implies Rietveld patch.
+      # TODO(tandrii): fix this madness.
+      issue = patchset = None
 
     # Point to the oauth2 auth files if specified.
     # These paths are where the bots put their credential files.