Yingchun Lai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18018 )

Change subject: [build] Introduce an env variable to indicate the thirdparty 
path
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@13
PS2, Line 13: paths,
> nit: paths
Done


http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@15
PS2, Line 15: - Build an docker image with thirdparty libraries well built, 
then we
> Can you elaborate on this? Is there a problem with thirdparty in our Docker
The Kudu docker doc [1] says images like 
'apache/kudu:thirdparty-[OS]-[VERSION]' are provided, but I didn't see it on 
DockerHub [2], seems the doc is outdated. On the other hand, currently, the 
3rdparty path and Kudu code path must be in the same root path, this patch can 
resolve this situaction.

1. https://github.com/apache/kudu/blob/master/docker/README.adoc
2. https://hub.docker.com/r/apache/kudu


http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py
File build-support/iwyu.py:

http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py@33
PS2, Line 33: from kudu_util import get_upstream_commit, check_output, ROOT, 
Colors, \
> nit: line is too long
Done


http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:

http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py@88
PS2, Line 88: sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
> isn't there a cleaner way to import a module from a parent directory?
improved a little...


http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py
File src/kudu/scripts/dump_breakpad_symbols.py:

http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@91
PS2, Line 91:   return env['THIRDPARTY_DIR'] if env.has_key('THIRDPARTY_DIR') 
else \
> nit: line is too long
Done


http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@106
PS2, Line 106:     # TODO: Use dump_syms_mac if on macOS.
> nit: line is too long
Done



--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Comment-Date: Fri, 12 Nov 2021 02:12:03 +0000
Gerrit-HasComments: Yes

Reply via email to