David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/8456 )
Change subject: IMPALA-6148: Specifying thirdparty deps as URLs ...................................................................... Patch Set 4: Code-Review+2 (1 comment) > Patch Set 4: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/8456/2/bin/bootstrap_toolchain.py@112 PS2, Line 112: if re.search(k, release): > The 57 times is why we're adding the cache. It was spamming the log, mostly Got it. Thanks for pointing that line out. It does seem to me that baking in platform as an attribute of Package fits the intention of the class -- perhaps with a default value, but override-able such that L240 might look like: Package("kudu", kudu_version, "centos7") But I get not wanting to get into excessive refactoring of existing code, and for now not spamming the log is an improvement, so I think what you have here is fine. -- To view, visit http://gerrit.cloudera.org:8080/8456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4628d86022d4bd8b762313f7056d76416a58b422 Gerrit-Change-Number: 8456 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-Comment-Date: Wed, 08 Nov 2017 21:58:18 +0000 Gerrit-HasComments: Yes
