Conversation
Co-authored-by: jkotas <[email protected]>
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
Co-authored-by: jkotas <[email protected]>
jkotas
left a comment
jkotas
left a comment
There was a problem hiding this comment.
@tannergooding We have a hard dependency on Win8+ after the recent change so IsWindows8OrAbove condition can be deleted
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
There was a problem hiding this comment.
Pull request overview
This PR removes the obsolete Environment.IsWindows8OrAbove internal property, which always returned true since Windows 8 is now the minimum supported version. The cleanup eliminates dead code paths that were unreachable on currently supported platforms.
Key changes:
- Removed
IsWindows8OrAboveproperty andWindowsVersionhelper class from Environment.Win32.cs - Removed Windows 7/Server 2008 R2 workarounds in CompareInfo.Nls.cs (3 conditional blocks for LCMapStringEx buffer handling)
- Simplified conditional logic in EventSource.cs by removing an always-true condition
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Environment.Win32.cs | Removed the IsWindows8OrAbove property definition and the WindowsVersion helper class |
| CompareInfo.Nls.cs | Removed 3 Windows 7/Server 2008 R2 specific workarounds for LCMapStringEx buffer handling that were no longer executed |
| EventSource.cs | Removed conditional check and made SetInformation call unconditional (the condition was always true) |
The code changes are clean and correct. All removed code paths were indeed unreachable since IsWindows8OrAbove always returned true. The removal of the Windows 7/Server 2008 R2 workarounds is appropriate, and the EventSource.cs logic simplification correctly reflects that the original condition (this.Name != "System.Diagnostics.Eventing.FrameworkEventSource" || Environment.IsWindows8OrAbove) was always true. No issues found.
|
I take it I can start opening pull requests for #71075? |
Yes, these cleanups can proceed now. |
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Show resolved
Hide resolved
Co-authored-by: jkotas <[email protected]>
|
/ba-g wasm timeout without a log |
Removed
Environment.IsWindows8OrAboveinternal property and related Windows 7 compatibility code, which is obsolete since Windows 7 is no longer supported.