Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18977 )
Change subject: IMPALA-10408: Support build using Apache components ...................................................................... Patch Set 21: (9 comments) Thanks for updating this patch! Just have some comments on the code format. http://gerrit.cloudera.org:8080/#/c/18977/21//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18977/21//COMMIT_MSG@9 PS21, Line 9: apache nit: "CDP" ? http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@a523 PS21, Line 523: nit: please keep the original indention http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@513 PS21, Line 513: hadoop = ApacheComponent("hadoop", : component_path_tmpl="${name}/common/${name}-${version}/", : archive_basename_tmpl="${name}-${version}") nit: move "hadoop" to the next line and use 4 spaces indention, i.e. hadoop = ApacheComponent( "hadoop", component_path_tmpl="${name}/common/${name}-${version}/", archive_basename_tmpl="${name}-${version}") Or make the arguments aligned, i.e. hadoop = ApacheComponent("hadoop", component_path_tmpl="${name}/common/${name}-${version}/", archive_basename_tmpl="${name}-${version}") http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@520 PS21, Line 520: hbase = ApacheComponent("hbase", : component_path_tmpl="${name}/${version}/", : archive_basename_tmpl="${name}-${version}-bin", : unpack_directory_tmpl="${name}-${version}") nit: make the arguments aligned http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@525 PS21, Line 525: hbase = CdpComponent("hbase", archive_basename_tmpl="hbase-${version}-bin", : unpack_directory_tmpl="hbase-${version}") nit: make the arguments aligned. http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@545 PS21, Line 545: tez = ApacheComponent("tez", : component_path_tmpl="${name}/${version}/", : archive_basename_tmpl="apache-${name}-${version}-bin") : else: : tez = CdpComponent("tez", : archive_basename_tmpl="tez-${version}-minimal", : makedir=True) : if use_apache_ranger: : ranger = ApacheComponent("ranger", : component_path_tmpl="${name}/${version}/", : archive_basename_tmpl="apache-${name}-${version}") nit: need to format the indention http://gerrit.cloudera.org:8080/#/c/18977/21/bin/bootstrap_toolchain.py@620 PS21, Line 620: nit: use 4 spaces indention http://gerrit.cloudera.org:8080/#/c/18977/21/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/18977/21/bin/impala-config.sh@996 PS21, Line 996: export RANGER_HOME=\ : ${RANGER_HOME_OVERRIDE:-\ : "${APACHE_COMPONENTS_HOME}/apache-ranger-${IMPALA_RANGER_VERSION}"} nit: format this to export RANGER_HOME=${RANGER_HOME_OVERRIDE:-\ "${APACHE_COMPONENTS_HOME}/apache-ranger-${IMPALA_RANGER_VERSION}"} http://gerrit.cloudera.org:8080/#/c/18977/15/java/TableFlattener/pom.xml File java/TableFlattener/pom.xml: http://gerrit.cloudera.org:8080/#/c/18977/15/java/TableFlattener/pom.xml@67 PS15, Line 67: <dependency> : <groupId>commons-lang</groupId> : <artifactId>commons-lang</artifactId> : <version>${commons-io.version}</version> : </dependency> > Without this, I failed to build it. Ack. Thanks for the explanation! -- To view, visit http://gerrit.cloudera.org:8080/18977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8730dd182b367c9daa94303937ad249db72b1399 Gerrit-Change-Number: 18977 Gerrit-PatchSet: 21 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Thu, 08 Aug 2024 02:43:56 +0000 Gerrit-HasComments: Yes
