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

Unified Diff: appengine/findit/common/deps_parser.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/deps_parser.py
diff --git a/appengine/findit/common/deps_parser.py b/appengine/findit/common/deps_parser.py
index b2b80b5639bd864814a9cfa7c176ac9f322b563b..78435261b6d00dae389d63f08bc81227194970fd 100644
--- a/appengine/findit/common/deps_parser.py
+++ b/appengine/findit/common/deps_parser.py
@@ -165,12 +165,18 @@ def UpdateDependencyTree(root_dep, target_os_list, deps_loader):
else:
target_os_list = [_NormalizeTargetOSName(name) for name in target_os_list]
+ # TODO(wrengr): why not just pass the deps_loader to a method on
+ # root_dep which returns the deps_content? What's the point in threading
+ # things through like this?
deps_content = deps_loader.Load(
root_dep.deps_repo_url, root_dep.deps_repo_revision, root_dep.deps_file)
deps, deps_os = ParseDEPSContent(deps_content, keys=('deps', 'deps_os'))
all_deps = MergeWithOsDeps(deps, deps_os, target_os_list)
+ # TODO(wrengr): why close over root_dep to get at deps_file, but then
+ # force callsers to patch things up after the fact with SetParent? Why
+ # not just call SetParent ourselves?
def _CreateDependency(path, repo_info):
if not path.endswith('/'):
path = path + '/'
@@ -189,6 +195,9 @@ def UpdateDependencyTree(root_dep, target_os_list, deps_loader):
# The dependency is not needed for all the target os.
continue
+ # TODO(wrengr): why are we doing this? i.e., it looks like we're
+ # allocating a dependency.Dependency object and then just dropping it
+ # on the floor. To what end?
sub_dep = _CreateDependency(path, repo_info)
sub_dep.SetParent(root_dep)

Powered by Google App Engine
This is Rietveld 408576698