Skip to content

Conversation

@gurry
Copy link
Contributor

@gurry gurry commented Dec 23, 2025

Fixes #580

Copilot AI review requested due to automatic review settings December 23, 2025 13:24
@gurry gurry requested a review from a team December 23, 2025 13:25
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 adds proper warning messages when WDK environment variables contain non-UTF8 characters, addressing issue #580. Previously, such errors were only logged at trace level without distinguishing between missing variables and encoding issues.

Key Changes:

  • Added warning-level logging for non-UTF8 environment variables in get_path_from_env
  • Distinguished between NotPresent and NotUnicode error variants with appropriate log levels

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

Copilot AI review requested due to automatic review settings December 23, 2025 13:26
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

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


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

@gurry gurry requested a review from wmmc88 December 23, 2025 13:28
@gurry gurry enabled auto-merge December 23, 2025 13:28
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.66%. Comparing base (dd8fdff) to head (1a3cb77).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/wdk-build/src/cargo_make.rs 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #596      +/-   ##
==========================================
- Coverage   82.67%   82.66%   -0.01%     
==========================================
  Files          25       25              
  Lines        7135     7137       +2     
  Branches     7135     7137       +2     
==========================================
+ Hits         5899     5900       +1     
- Misses       1107     1108       +1     
  Partials      129      129              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

wmmc88
wmmc88 previously approved these changes Dec 24, 2025
Copy link
Collaborator

@wmmc88 wmmc88 left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor nit. Merge is going to have to wait for #597 and probably #598 to go in

wmmc88
wmmc88 previously approved these changes Dec 25, 2025
Copilot AI review requested due to automatic review settings December 27, 2025 06:16
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

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


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

@gurry gurry dismissed stale reviews from krishnakumar4a4 and wmmc88 via 1a3cb77 December 27, 2025 06:50
@gurry gurry added this pull request to the merge queue Dec 27, 2025
Merged via the queue into microsoft:main with commit c3c1223 Dec 27, 2025
252 of 253 checks passed
@gurry gurry deleted the 580-warn-on-invalid-var branch December 27, 2025 19:41
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.

Warn on invalid values for WDK Env Vars

4 participants