Copilot commented on code in PR #11712:
URL: https://github.com/apache/gluten/pull/11712#discussion_r3391317761


##########
ep/build-velox/src/get-velox.sh:
##########
@@ -17,8 +17,8 @@
 set -exu
 
 CURRENT_DIR=$(cd "$(dirname "$BASH_SOURCE")"; pwd)
-VELOX_REPO=https://github.com/IBM/velox.git
-VELOX_BRANCH=dft-2026_06_06
+VELOX_REPO=https://github.com/zhouyuan/velox.git
+VELOX_BRANCH=dft-2026_06_06-fbthrift

Review Comment:
   Defaulting VELOX_REPO to a personal fork makes builds less reproducible and 
diverges from the project’s documented Velox fork (IBM/velox). It also likely 
breaks `--enable_enhanced_features` defaults (the `ibm-*` branch naming 
suggests those refs exist only in the IBM fork). Consider keeping IBM/velox as 
the default and using `--velox_repo/--velox_branch` flags when testing a 
temporary fork/branch.



##########
dev/vcpkg/vcpkg.json:
##########
@@ -24,6 +24,8 @@
           "name": "folly",
           "features": ["zstd", "lz4", "lzma", "snappy"]
         },
+             "fizz",

Review Comment:
   This line is indented with a tab, which is inconsistent with the rest of the 
JSON (spaces) and can cause noisy diffs or formatting/lint failures. Use spaces 
for indentation here.



##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -77,7 +77,7 @@ jobs:
             export CCACHE_MAXSIZE=1G
             mkdir -p /work/.ccache
             ccache -sz
-            bash dev/ci-velox-buildstatic-centos-7.sh
+            bash dev/ci-velox-buildstatic-centos-9.sh
             ccache -s

Review Comment:
   The job/artifact naming and downstream tests indicate the native libs are 
intended to be built for CentOS 7 compatibility (and are later consumed inside 
a CentOS 7 container). Building them in a CentOS 9-based image can introduce a 
newer glibc/libstdc++ ABI and break runtime on CentOS 7/8. If the intent is to 
keep CentOS 7-compatible artifacts, this step should continue using the CentOS 
7 build image and script; otherwise the entire workflow (job/artifact naming 
and CentOS 7/8 test jobs) needs to be updated together.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to