-
Notifications
You must be signed in to change notification settings - Fork 192
CI/CD fix #429
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
CI/CD fix #429
Conversation
ubuntu-22.04 and 24.04
- Using Ubuntu 22.04 and Ubuntu 24.04 - corresponding supported compilers - Properly installing docker - CompareRecursive fix for GCC-12 (fixes ClickHouse#425)
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.
Pull Request Overview
This PR updates CI/CD workflows to support newer Ubuntu versions and compilers, improves Docker installation steps, and enhances the CompareRecursive utility with additional tests and type handling.
- Expanded
CompareRecursivetests to cover characters, string types, and containers of strings - Refactored
utils_comparison.hto introduceL/Rtype aliases and handlechar*viastd::string_view - Updated GitHub Actions across Windows, macOS, and Linux: added wildcard branch triggers, new compiler matrix entries, and Docker verification
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ut/utils_ut.cpp | Added new tests for CompareRecursive to cover char, string, and container cases |
| ut/utils_meta.h | Minor formatting cleanup (added blank line) |
| ut/utils_comparison.h | Introduced using L/R aliases and new char* overloads |
| .github/workflows/windows_msvc.yml | Changed push trigger to all branches |
| .github/workflows/windows_mingw.yml | Changed push trigger to all branches |
| .github/workflows/macos.yml | Changed push trigger to all branches |
| .github/workflows/linux.yml | Changed push trigger to all branches; revamped compiler matrix and Docker steps |
Comments suppressed due to low confidence (6)
.github/workflows/windows_msvc.yml:7
- [nitpick] The push trigger is now on all branches, but pull_request remains limited to master; consider aligning pull_request to '*' so feature-branch PRs also run CI.
branches: [ '*' ]
.github/workflows/windows_mingw.yml:7
- [nitpick] The push trigger is now on all branches, but pull_request remains limited to master; consider aligning pull_request to '*' so feature-branch PRs also run CI.
branches: [ '*' ]
.github/workflows/macos.yml:7
- [nitpick] The push trigger is now on all branches, but pull_request remains limited to master; consider aligning pull_request to '*' so feature-branch PRs also run CI.
branches: [ '*' ]
.github/workflows/linux.yml:7
- [nitpick] The push trigger is now on all branches, but pull_request remains limited to master; consider aligning pull_request to '*' so feature-branch PRs also run CI.
branches: [ '*' ]
.github/workflows/linux.yml:25
- The top-level matrix no longer defines an
osaxis, which may lead to runs with an undefined OS; reintroduceos: [ubuntu-22.04, ubuntu-24.04]or ensure every matrix entry specifiesos.
compiler: [clang-11, clang-14, clang-18, gcc-9, gcc-12, gcc-14]
ut/utils_ut.cpp:91
- Lines 91–92 duplicate the same assertions from lines 89–90; consider removing these redundant tests to keep the suite concise.
EXPECT_TRUE(CompareRecursive(vector_of_strings, vector_of_string_views));
Mostly focused on fixing CI/CD on Linux
Closes #415 (supersedes)