Skip to content

Conversation

@gargsaumya
Copy link
Contributor

@gargsaumya gargsaumya commented Jun 16, 2025

This pull request introduces a platform-specific enhancement to the build configuration for ddbc_bindings in the CMakeLists.txt file. It adds stricter warning level flags for MSVC to improve code quality.

Build configuration improvement:

  • mssql_python/pybind/CMakeLists.txt: Added a conditional block to apply /W4 (warning level 4) and /WX (treat warnings as errors) compiler flags when building with MSVC. This ensures stricter code quality checks for Windows builds.

Copilot AI review requested due to automatic review settings June 16, 2025 09:39
@gargsaumya gargsaumya force-pushed the saumya/no-level-3-4-warning branch from ef57050 to a2e5c9f Compare June 16, 2025 09:40
Copy link
Contributor

Copilot AI left a 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 tightens MSVC build settings by treating warnings as errors for the ddbc_bindings target.

  • Adds /W4 and /WX compiler flags when using MSVC
  • Improves code quality enforcement on Windows builds
Comments suppressed due to low confidence (1)

mssql_python/pybind/CMakeLists.txt:290

  • The MSVC CMake variable doesn’t cover clang-cl in MSVC compatibility mode. Consider using a generator expression like if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" OR (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "MSVC")) to catch both.
if(MSVC)

@gargsaumya gargsaumya changed the title FEAT: warnings should be caught as error FEAT: Enforce Level 3 and 4 warnings as errors to adhere to compliance standards. Jun 16, 2025
@gargsaumya gargsaumya force-pushed the saumya/no-level-3-4-warning branch from a2e5c9f to 37e3407 Compare June 16, 2025 09:47
@bewithgaurav bewithgaurav merged commit 05560ef into main Jun 16, 2025
4 checks passed
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.

4 participants