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

Change subject: KUDU-3788 an option to use pre-built thirdparty artifacts
......................................................................


Patch Set 11:

(6 comments)

> (6 comments)
 >
 > Thank you for addressing the comments.
 >
 > Almost there. Just a few nits.
 >
 > I am not sure if docker builds are automatically tested by
 > dist-cluster or has to be done explicitly. Would be great if you
 > could verify that everything checks out for docker build. Btw, I am
 > fine with addressing any unforeseen issues there in a separate
 > change.

Yep, I saw that AI tools provided code review feedback on so-called Docker 
build when asked for review, but I'm not sure that's relevant at all.  If 
anything comes up, I'm happy to address that in a follow-up changelist -- it's 
too much packed here already, and I was even thinking to start splitting this 
patch into pieces :)

http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh
File thirdparty/prebuilt-utils.sh:

http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@80
PS11, Line 80: cut -d. -f1
> I may have highlighted wrong place that caused the confusion. I should have
Done


http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh
File thirdparty/prebuilt-utils.sh:

http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@116
PS12, Line 116: $version_str
> Should this be in quotes to avoid collapse of newlines to spaces and always
Good catch -- indeed, quotes were missing here.


http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@253
PS12, Line 253: Creating pre-b
> This is unused, can be removed here and in callers.
Done


http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@381
PS12, Line 381:
> s/$tarball/$tarball_name ?
Done


http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@388
PS12, Line 388: # Cleanup the staging sub-direc
> $staging_root will always be non-empty with '/' lying around even if $compo
Done


http://gerrit.cloudera.org:8080/#/c/24583/12/thirdparty/prebuilt-utils.sh@413
PS12, Line 413: tall_sub
> s/$tarball/$tarball_name ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55b5b99fcbe60eec3f6ae19fa4f2e335f361d143
Gerrit-Change-Number: 24583
Gerrit-PatchSet: 11
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Thu, 30 Jul 2026 18:49:00 +0000
Gerrit-HasComments: Yes

Reply via email to