Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: WIP: Add an intial docker integration ...................................................................... Patch Set 1: (9 comments) Neat stuff! http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@20 PS1, Line 20: - Add kudu-master and kudu-tserver images with ENTRYPOINT’s Is this an incomplete thought or is the apostrophe in ENTRYPOINT unnecessary? http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@1 PS1, Line 1: # Nit: remove empty line? http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@22 PS1, Line 22: edditing editing http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@27 PS1, Line 27: Nit: two spaces here http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@35 PS1, Line 35: FROM centos:7.6.1810 as kudu-base Curious why you chose to start with CentOS 7.6 and something like 6.6 (which would be the oldest we support). Maybe document that decision in the commit message? http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@50 PS1, Line 50: gdb git \ Nit: looks like you're listing one dependency per line except here and in a few places below. Could you fix that? http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@88 PS1, Line 88: COPY ./thirdparty thirdparty Does COPY operate recursively? Will it grab thirdparty/patches, for example? http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@100 PS1, Line 100: ARG PARALLEL=4 Is it safe to run `nproc` here to derive the parallelism? http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh@746 PS1, Line 746: mkdir -p $TP_DIR/../www/ Curious why this is needed: the 'www' subdirectory is checked into source control (well, a bunch of files in it are checked in, but you get the idea). -- 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: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 08 Jan 2019 06:07:35 +0000 Gerrit-HasComments: Yes
