Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16885 )
Change subject: [docker] Add support for openSUSE in the Docker build ...................................................................... Patch Set 2: (20 comments) http://gerrit.cloudera.org:8080/#/c/16885/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16885/2//COMMIT_MSG@15 PS2, Line 15: > nit: remove space Done http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh File docker/bootstrap-dev-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@185 PS2, Line 185: zypper > Does it make sense to add --non-interactive option to mirror what's done fo I don't think we have a license issue given it builds with no problems. The same for interactive modes. I guess this is a case of YAGNI. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@185 PS2, Line 185: update > According to https://en.opensuse.org/images/1/17/Zypper-cheat-sheet-1.pdf update does a refresh and update and matches the behavior of the other OS variants. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@209 PS2, Line 209: openssl-devel > Why openssl isn't enough? This was taken from the install instructions: https://kudu.apache.org/docs/installation.html#sles_from_source http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@220 PS2, Line 220: exta > nit: extra Done http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@221 PS2, Line 221: tzdata equivalent package. > What doesn't work not having this? Could you mention that either in the com Done http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-dev-env.sh@231 PS2, Line 231: vim > The 'vim' package is already in the fist list, no? Done http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh File docker/bootstrap-java-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh@76 PS2, Line 76: update > refresh ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh@76 PS2, Line 76: zypper > Add --non-interactive ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh@79 PS2, Line 79: install > Add --non-interactive and --auto-agree-with-licenses ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-java-env.sh@82 PS2, Line 82: clean > Add --all ? for what? http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-python-env.sh File docker/bootstrap-python-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-python-env.sh@104 PS2, Line 104: zypper > here and below: add --non-interactive flag for zypper? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-python-env.sh@106 PS2, Line 106: update > refresh ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-python-env.sh@117 PS2, Line 117: clean > clean --all ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh File docker/bootstrap-runtime-env.sh: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@86 PS2, Line 86: zypper > here and below: add --non-interactive ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@90 PS2, Line 90: install > add --auto-agree-with-licenses ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@95 PS2, Line 95: openssl > Should this be openssl-devel ? It doesn't appear to be required for runtime. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@99 PS2, Line 99: install > add --auto-agree-with-licenses ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/bootstrap-runtime-env.sh@105 PS2, Line 105: clean > clean --all ? see my other comment on this. http://gerrit.cloudera.org:8080/#/c/16885/2/docker/docker-build.py File docker/docker-build.py: http://gerrit.cloudera.org:8080/#/c/16885/2/docker/docker-build.py@165 PS2, Line 165: if "/" in base: > nit: it would be nice to add an example for the 'base' which requires extra Done -- To view, visit http://gerrit.cloudera.org:8080/16885 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c1ce282fda67db5043445a139c888bc74c44680 Gerrit-Change-Number: 16885 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Dec 2020 18:25:43 +0000 Gerrit-HasComments: Yes
