Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 6, 2025

This PR removes redundant #if NET conditional compilation directives from files that are only compiled in .NET builds, making the code cleaner and more maintainable.

Files Changed

Exception Classes (aspnetcore shared folder):

  • HuffmanDecodingException.cs - removed 3 redundant #if NET blocks around [Obsolete] attributes
  • QPackEncodingException.cs - removed 1 redundant #if NET block around [Obsolete] attribute
  • QPackDecodingException.cs - removed 1 redundant #if NET block around [Obsolete] attribute

Network Exception Class:

  • NetworkInformationException.cs - removed 1 redundant #if NET block around [Obsolete] attribute

.NET-Only Reference Files:

  • System.Text.Json.netcoreapp.cs - removed 1 redundant #if NET block around Int128/UInt128 converter properties
  • System.Security.Permissions.netcoreapp.cs - removed 1 redundant #if NET block around XamlLoadPermission obsolete attribute

Rationale

These files are only included in .NET builds where the NET symbol is always defined:

  • The aspnetcore HTTP exception classes only target $(NetCoreAppCurrent) and its platform-specific variants
  • NetworkInformationException.cs is in Common folder but only used in .NET-only contexts
  • .netcoreapp.cs files are conditionally included only when '$(TargetFrameworkIdentifier)' == '.NETCoreApp'

Since the NET symbol is always defined in these build contexts, the #if NET conditionals are always true and therefore redundant.

Testing

  • Built affected libraries successfully for net10.0
  • All [Obsolete] attributes and properties are now always applied as intended
  • No functional changes - only removes unnecessary conditional compilation

Fixes #118421.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Remove redundant #if NET directives Remove redundant #if NET directives from .NET-only exception classes Aug 6, 2025
@Copilot Copilot AI requested a review from jkotas August 6, 2025 03:13
Copilot finished work on behalf of jkotas August 6, 2025 03:13
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM

@jkotas jkotas marked this pull request as ready for review August 6, 2025 04:00
@Copilot Copilot AI review requested due to automatic review settings August 6, 2025 04:00
Copy link
Contributor

@Copilot 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 removes redundant #if NET conditional compilation directives from exception classes that are only compiled in .NET builds, making the code cleaner and more maintainable.

  • Removed #if NET blocks around [Obsolete] attributes for legacy serialization methods
  • Changes affect 4 exception classes in HTTP and network libraries
  • No functional changes - the [Obsolete] attributes are now always applied as intended

Reviewed Changes

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

File Description
NetworkInformationException.cs Removed 1 redundant #if NET block around obsolete serialization constructor
QPackEncodingException.cs Removed 1 redundant #if NET block around obsolete serialization constructor
QPackDecodingException.cs Removed 1 redundant #if NET block around obsolete serialization constructor
HuffmanDecodingException.cs Removed 3 redundant #if NET blocks around obsolete serialization constructor and GetObjectData methods

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@rzikm rzikm requested a review from a team August 6, 2025 06:51
ManickaP
ManickaP previously approved these changes Aug 6, 2025
@xtqqczze
Copy link
Contributor

xtqqczze commented Aug 6, 2025

@copilot Your analysis currently focuses on instances of the "#if NET" pattern. To ensure a more comprehensive evaluation, consider generalizing your search logic to detect all redundant or unnecessary uses of the NET symbol, including inverse conditions such as #if !NET and other conditional forms.

@jkotas
Copy link
Member

jkotas commented Aug 6, 2025

@xtqqczze Copilot won't listed to external contributors.

@jkotas
Copy link
Member

jkotas commented Aug 6, 2025

@copilot Your analysis currently focuses on instances of the "#if NET" pattern. To ensure a more comprehensive evaluation, consider generalizing your search logic to detect all redundant or unnecessary uses of the NET symbol, including inverse conditions such as #if !NET and other conditional forms.

Copy link
Contributor Author

Copilot AI commented Aug 6, 2025

@copilot Your analysis currently focuses on instances of the "#if NET" pattern. To ensure a more comprehensive evaluation, consider generalizing your search logic to detect all redundant or unnecessar...

Expanded the search beyond #if NET patterns as requested. Found and removed redundant NET symbol usage in two additional .NET-only files:

  • System.Text.Json.netcoreapp.cs - removed #if NET around Int128/UInt128 converters
  • System.Security.Permissions.netcoreapp.cs - removed #if NET around XamlLoadPermission obsolete attribute

These .netcoreapp.cs files are only compiled for .NETCoreApp targets where the NET symbol is always defined, making the conditionals redundant. Both files build successfully after the changes.

Commit: 3db9939

@Copilot Copilot AI changed the title Remove redundant #if NET directives from .NET-only exception classes Remove redundant #if NET directives from .NET-only files Aug 6, 2025
@Copilot Copilot AI requested a review from jkotas August 6, 2025 13:52
Copilot finished work on behalf of jkotas August 6, 2025 13:52
@jkotas
Copy link
Member

jkotas commented Aug 6, 2025

/ba-g intermittent network issue

@jkotas jkotas merged commit 8c7de74 into main Aug 6, 2025
79 of 82 checks passed
@jkotas jkotas deleted the copilot/fix-118421 branch August 6, 2025 17:55
@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove redundant #if NET directives
6 participants