Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build ...................................................................... Patch Set 7: (10 comments) http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@10 PS7, Line 10: various Docker : images > Does it make sense to add more specifics about the currently supported plat It's in the code and "supported" isn't really a thing yet anyway given this is experimental work. http://gerrit.cloudera.org:8080/#/c/12173/7//COMMIT_MSG@14 PS7, Line 14: wih > with? Done http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31 PS7, Line 31: TODO > We don't have a style guide for Dockerfiles, but maybe follow the same styl I actually really dislike the TODO(user) model. Given this is an open source project and no individual person is responsible for the code I don't think a name should be tied to a todo. It's also really easy to see who added it via git blame if that is the need. http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@31 PS7, Line 31: seperate > separate Done http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@51 PS7, Line 51: > nit: maybe, add an extra indent for this line since it's a continuation of Done http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@63 PS7, Line 63: it's > its Done http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@79 PS7, Line 79: /* > nit: drop ? Or you want to keep subdirs like .x ? Not sure what you mean. This isn't leaving any subdirectories other than the hadoop, hive, and sentry ones. http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@84 PS7, Line 84: /* > ditto See above. http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@116 PS7, Line 116: ARG PARALLEL=4 > Maybe, just rely on GNU make and run as many parallel tasks as it allows by See related comment below. http://gerrit.cloudera.org:8080/#/c/12173/7/docker/Dockerfile@138 PS7, Line 138: -j${PARALLEL} > As commented above, maybe just leave '-j' and run as many parallel tasks as Lets experiment with this in future patches. -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 25 Jan 2019 20:42:18 +0000 Gerrit-HasComments: Yes
