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

Change subject: [thirdparty] fix building LLVM/CLANG 11.0.0 with GCC13
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@15
PS1, Line 15: change
> nit: that
Done


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@16
PS1, Line 16: leaking in
> nit: leakage of
Done


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@17
PS1, Line 17: installed at the build machine
> nit: installed on the build machine
Done


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@21
PS1, Line 21: codegen
> I guess we can confirm that by running multiple iterations of modified code
My SLES15 virtual machine has been destroyed recently, so it would be an extra 
effort to recreate one.

Yes, if you have some spare cycles and active/existing development environment 
on SLES15 machine, it would be great to verify this hypothesis once this patch 
is committed/pushed.  No pressure, though.


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@22
PS1, Line 22: this hypothesis
> Maybe mention this patch in those Jiras for the Jira reporters to be aware
That's done by gerrit automation itself: once KUDU-xxx pattern is seen, there 
will be an comment posted at corresponding Jira items.


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@27
PS1, Line 27: Ubuntu 24.04 LTS
> Any other OS flavour (other than Ubuntu 24.04 LTS) where this patch is appl
I haven't explored anything else beyond Ubuntu 24.04 LTS and Ubuntu 24.04.1 
LTS.  The idea was to make sure the upcoming Apache Kudu 1.18.0 release could 
be built on not-so-long releases Ubuntu LTS release.


http://gerrit.cloudera.org:8080/#/c/22279/1//COMMIT_MSG@29
PS1, Line 29: a
> an
Done


http://gerrit.cloudera.org:8080/#/c/22279/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

http://gerrit.cloudera.org:8080/#/c/22279/1/thirdparty/download-thirdparty.sh@334
PS1, Line 334: 9
> Why did we jump patch level? isn't it supposed to be simply sequential inst
Right -- that's exactly so.  I changed this to be the number of patches since 
it was helpful while I was adding more and more patches in my workspace.  But I 
forgot to revert it back to the 'sequential' semantics.  Thanks for pointing it 
out.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b8282c85bbac5f25eae885e4b3ec183104bc540
Gerrit-Change-Number: 22279
Gerrit-PatchSet: 1
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: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Wed, 15 Jan 2025 19:02:25 +0000
Gerrit-HasComments: Yes

Reply via email to