Skip to content

[ci] no need for the windows generator - #187

Open
simogasp wants to merge 4 commits into
developfrom
ci/fix_win
Open

[ci] no need for the windows generator#187
simogasp wants to merge 4 commits into
developfrom
ci/fix_win

Conversation

@simogasp

Copy link
Copy Markdown
Member

Build was failing on Windows due to the switch to the new version of windows latest.
When using vcpkg, there is no need to explicitly specify the generator and the architecture; it will be resolved automatically.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request removes the explicit Visual Studio generator and the build type configuration from the CMake commands in the Windows build script. The feedback correctly points out that removing the build type configuration can cause issues with single-configuration generators like Ninja, where the build type must be specified during the configuration step.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 62 to 63
-DPopSift_BUILD_EXAMPLES:BOOL=ON `
-DCMAKE_BUILD_TYPE="$BuildType" `
-DCMAKE_INSTALL_PREFIX="$installDir" `

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Removing -DCMAKE_BUILD_TYPE="$BuildType" can cause issues if CMake resolves to a single-configuration generator (such as Ninja). For single-configuration generators, the build type must be specified during the configuration step; otherwise, it defaults to Debug or empty, and subsequent build commands (e.g., cmake --build . --config $BuildType) will not build the expected configuration. It is safer to keep this flag to ensure compatibility with both single-config and multi-config generators.

      -DPopSift_BUILD_EXAMPLES:BOOL=ON `
      -DCMAKE_BUILD_TYPE="$BuildType" `
      -DCMAKE_INSTALL_PREFIX="$installDir" `

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant