-
Notifications
You must be signed in to change notification settings - Fork 5.5k
chore(ci): Advance velox #26665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(ci): Advance velox #26665
Conversation
amitkdutta
commented
Nov 20, 2025
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR advances the Velox submodule to the latest upstream commit, updating the local reference and ensuring the build picks up recent fixes and improvements from Velox. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@amitkdutta Found the issue. PR facebookincubator/velox#15284 added code that inserts an error message to the license-check.py if we want to remove the header. And we are regenerating the protocol in github and so it triggers the message to be added to the output file causing the compile errors. |
|
@amitkdutta The issue is that it uses the Velox license script which might change behavior for Velox purposes. Presto-native-execution has its own license check and this should be used to remove the header from the special protocol files. Edit: PrestoC++ CI is ok now. Only temp issues with some of the Java tests. Removing the test commit. Need to rebase once the fix merges. |
|
@amitkdutta The Presto branch was updated and I rebased now. CI should be clean now. |
|
Thanks @amitkdutta, could you please rebase Velox to the latest commit, it would be helpful to include this change. |