Enable building of NVBench on Windows#362
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR adds complete Windows build infrastructure to NVBench's CI system. It defines an MSVC2022 compiler configuration in the matrix, creates a reusable ChangesWindows CI Build Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pr.yml:
- Around line 80-99: The nvbench-windows job is currently blocking merges while
failures are expected; make it non-blocking by adding continue-on-error: true to
the nvbench-windows job definition (the job named "nvbench-windows") and also
remove nvbench-windows from the dependency/needs list of the final "ci" job (or
otherwise avoid marking it as a required check) so the ci job does not treat
nvbench-windows as a blocking prerequisite. Ensure you edit the nvbench-windows
job block to include continue-on-error: true and update the "ci" job's needs so
it no longer depends on nvbench-windows.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 6563790c-1254-423e-b4e7-1456b7a5ffb3
📒 Files selected for processing (6)
.github/actions/compute-matrix/compute-matrix.sh.github/workflows/build-windows.yml.github/workflows/pr.ymlci/matrix.yamlci/windows/build_common.psm1ci/windows/build_nvbench.ps1
|
Verified that job failed due to CMake failure, not due to issue with CI scripts: Ready for review @alliepiper |
No testing on Windows is intended.
This PR wants to add CI plumbing to enable testing of #354, but the Windows build in this PR is expected to fail.
The plan is
Then in #354, the main would be merged, windows build job enabled to permit iterations.
CC: @mfranzrebsal
Summary by CodeRabbit