Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1628)

Unified Diff: appengine/findit/common/rietveld.py

Issue 2344443005: [Findit] Factoring the gitiles (etc) stuff out into its own directory (Closed)
Patch Set: rebase-update Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: appengine/findit/common/rietveld.py
diff --git a/appengine/findit/common/rietveld.py b/appengine/findit/common/rietveld.py
index fab452cbd107029c3f3be7d7ac718975a4d3200a..d4b53abd12e756466724dd9c063ed555f21742c4 100644
--- a/appengine/findit/common/rietveld.py
+++ b/appengine/findit/common/rietveld.py
@@ -2,18 +2,28 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+# TODO(wrengr): this file is only used by waterfall. We should either
stgao 2016/10/13 05:47:32 This should go to an infra/ directory in the new c
wrengr 2016/10/27 17:28:00 Is there a bug ticket for doing the new layout? If
+# move it to that directory, or else factor out the dependency on
+# HttpClientAppengine by abstracting over the HTTP_CLIENT member. The
stgao 2016/10/13 05:47:32 Regarding the HTTP_CLIENT member, we could pass it
wrengr 2016/10/27 17:28:00 Acknowledged.
+# only thing we use/need HTTP_CLIENT for is to call the Post method,
+# which could just as well be rewritten to use the __call__ method
+# instead; hence, we could just pass an appropriate callable object to
+# the constructor (incidentally allowing multiple Rietveld objects to
+# share the same HTTP_CLIENT, if desired) rather than allocating our own
+# one automatically. Does rietveld itself actually require appengine?
+# Or is that just an incidental circumstance about our particular use
+# of it?
import logging
import re
import urlparse
-from common.codereview import CodeReview
stgao 2016/10/13 05:47:32 Please check my previous comment on why we wanted
from common.http_client_appengine import HttpClientAppengine
_RIETVELD_ISSUE_NUMBER_RE = re.compile('^/(\d+)/?.*')
-class Rietveld(CodeReview):
+class Rietveld(object):
"""The implementation of CodeReview interface for Rietveld."""
HTTP_CLIENT = HttpClientAppengine(follow_redirects=False)

Powered by Google App Engine
This is Rietveld 408576698