Skip to content

fix: define NOMINMAX for Windows addons - #12

Merged
gabewillen merged 1 commit into
mainfrom
feat/typescript-owned-prebuild-pipeline
Aug 7, 2026
Merged

gabewillen merged 1 commit into
mainfrom
feat/typescript-owned-prebuild-pipeline

Conversation

@gabewillen

Copy link
Copy Markdown
Contributor

Fixes the remaining Windows native matrix error: core telemetry uses std::max, which conflicts with the Windows max macro unless NOMINMAX is defined.

Adds /DNOMINMAX alongside /EHsc, keeps CORTEXT_WARNINGS_AS_ERRORS=OFF for the native orchestration, and preserves ARM clang-cl opt-in. The prior run failed before collection/publication.

Copilot AI lite review requested due to automatic review settings August 7, 2026 10:44
@gabewillen
gabewillen merged commit 5871ffe into main Aug 7, 2026
6 checks passed
@gabewillen
gabewillen deleted the feat/typescript-owned-prebuild-pipeline branch August 7, 2026 10:46

Copilot AI left a comment

Copy link
Copy Markdown

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 resolves a remaining Windows native build failure by ensuring NOMINMAX is defined for Windows addon builds, preventing Windows max/min macros from conflicting with C++ std::max usage during core telemetry compilation.

Changes:

  • Adds /DNOMINMAX to the Windows CMake C++ flags used by the prebuild orchestration script.
  • Updates the corresponding test assertion to match the new Windows CMake flag string.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
scripts/build-prebuild.mjs Appends /DNOMINMAX to -DCMAKE_CXX_FLAGS on Windows prebuild configuration.
tests/prebuilds.test.mjs Updates the regex assertion to reflect the new Windows CMake flag string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants