Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23848 )

Change subject: IMPALA-14942: [tools] Impala devcontainer
......................................................................


Patch Set 25:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh
File .devcontainer-build/build-devcontainer.sh:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh@50
PS25, Line 50: PLATFORM
What if PLATFORM isn't empty, but none of "linux/amd64" or "linux/arm64"?


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh@51
PS25, Line 51: uname -p
nit: using 'uname -m' might be more portable and future-proof

What if somebody runs this on macOS and 'uname -p'/'uname -m' reports x86_64 or 
arm64?  Is it consistent to assign PLATFORM="linux/xxx" then?


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh@54
PS25, Line 54: else
Shouldn't there be a catch-all-but x86_64/arm64 case to enforce 
allowable/acceptable PLATFORM, as per the comment?


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/devcontainer.json
File .devcontainer-build/devcontainer.json:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/devcontainer.json@43
PS25, Line 43:         "ms-vscode.cmake-tools",
             :         "vadimcn.vscode-lldb",
             :         "webfreak.debug",
             :         "vscjava.vscode-java-pack",
             :         "ms-python.python",
             :         "mohsen1.prettify-json",
             :         "alefragnani.bookmarks",
             :         "coolchyni.beyond-debug",
             :         "ms-vscode.cpptools",
             :         "ms-vscode.cpptools-extension-pack"
Are these really necessary or those are personal preferences?

Are these OK to use per ASF 3rd-party license policy? 
https://www.apache.org/legal/resolved.html


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer/regen-ssh-keys.sh
File .devcontainer/regen-ssh-keys.sh:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer/regen-ssh-keys.sh@38
PS25, Line 38: created '$(date)
There is date information encoded in the very first component of the comment 
already; is it necessary to duplicate it in locale-dependent format?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6aa834494003619fcdfbd98f3bfc7c74d3e1554c
Gerrit-Change-Number: 23848
Gerrit-PatchSet: 25
Gerrit-Owner: Jason Fehr <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Thu, 18 Jun 2026 18:26:49 +0000
Gerrit-HasComments: Yes

Reply via email to