Add CI workflow and per-configuration compiler flags#4
Conversation
📝 WalkthroughWalkthroughAdds a new GitHub Actions CI workflow building and testing across GCC, Clang, and MSVC configurations. Refactors CMakeLists.txt to apply optimization/LTO/architecture flags only in Release builds while keeping warnings always enabled. Updates .gitignore, README badge, build guidance, and roadmap. ChangesCI and Build Configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant Runner
participant CMake
participant CTest
GitHub->>Runner: trigger on push/pull_request
Runner->>Runner: set CC/CXX from matrix
Runner->>CMake: configure with build_type
Runner->>CMake: build --config --parallel
Runner->>CTest: run tests -C build_type
CTest-->>Runner: --output-on-failure results
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
1-54: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider adding ccache for faster CI builds.
For a C++ project, ccache can significantly speed up rebuilds by caching compiled object files. This is optional but straightforward to add for the Linux/macOS jobs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 54, Add ccache support to speed up repeated CI builds on the non-Windows jobs. Update the build-and-test job in the CI workflow so the ubuntu-latest and macos-latest matrix entries use ccache when configuring and building with CMake, while leaving the windows-latest entries unchanged. Use the existing matrix fields like name/os/build_type and the Configure/Build steps to wire in the cache-enabled compiler launcher.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 45: The checkout step in the CI workflow should disable GitHub token
persistence. Update the existing actions/checkout@v4 usage to set
persist-credentials to false so the workflow does not leave credentials in the
local git config; this change should be made in the checkout job step and kept
consistent with the rest of the build/test-only workflow.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1-54: Add ccache support to speed up repeated CI builds on the
non-Windows jobs. Update the build-and-test job in the CI workflow so the
ubuntu-latest and macos-latest matrix entries use ccache when configuring and
building with CMake, while leaving the windows-latest entries unchanged. Use the
existing matrix fields like name/os/build_type and the Configure/Build steps to
wire in the cache-enabled compiler launcher.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 651dfdba-422a-42e8-b317-12e40c1c2bff
📒 Files selected for processing (4)
.github/workflows/ci.yml.gitignoreCMakeLists.txtREADME.md
| CXX: ${{ matrix.cxx }} | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on checkout.
actions/checkout@v4 persists a GitHub token in the local git config by default. Since this workflow only builds and tests, there's no need for token persistence. Disabling it reduces the attack surface if a build step ever executes untrusted input.
🛡️ Proposed fix
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 45-45: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 45, The checkout step in the CI workflow
should disable GitHub token persistence. Update the existing actions/checkout@v4
usage to set persist-credentials to false so the workflow does not leave
credentials in the local git config; this change should be made in the checkout
job step and kept consistent with the rest of the build/test-only workflow.
Source: Linters/SAST tools
Optimization flags (including /O2, /GL, -O3, -flto, -ffast-math) now apply only to Release builds via generator expressions, so MSVC Debug builds no longer fail on the /O2 vs /RTC1 collision. -march=native is gated behind a compiler capability check for portability to older Apple Clang on arm64. The GitHub Actions matrix covers GCC/Clang on Ubuntu, Clang on macOS, and MSVC on Windows in both Release and Debug, running the full CTest suite.
f137ca3 to
452897a
Compare
Summary
/O2 /GL /arch:AVX2 /fp:faston MSVC;-O3 -flto -ffast-mathetc. on GCC/Clang) now apply only to Release builds via$<$<CONFIG:Release>:...>generator expressions. MSVC Debug builds previously failed outright on the/O2vs/RTC1collision; both configurations now build and pass all 51 tests.-march=nativeportability guard: gated behindcheck_cxx_compiler_flagso compilers that reject it (older Apple Clang on arm64) skip it instead of failing configure..github/workflows/ci.yml): GCC + Clang on Ubuntu, Clang on macOS, MSVC on Windows in both Release and Debug — the Debug job exists specifically to keep the per-config flags honest. Every job runs the full CTest suite.build-*/trees.Test plan
-O0, expectation documented in README)