Skip to content

Conversation

jaredpar
Copy link
Member

@jaredpar jaredpar commented Oct 3, 2025

Using Copilot CLI locally to make speculative changes to the sdk. The CLI was able to detect when multi-targeting was needed to access new .NET Core APIs but it defaulted to emitting #if NET6_0_OR_GREATER. Looking at the build files our minimum TFM is, likely, net9.0 hence this is unnecessary.

Considered for a bit adding an instruction that told copilot what the minimum TFM was but in the end decided that the preference would almost always be #if NET anyways hence just default to that.

Tested before / after locally this change resulted a better experience.

Using Copilot CLI locally to make speculative changes to the sdk. The CLI was able to detect when multi-targeting was needed to access new .NET Core APIs but it defaulted to emitting `#if NET6_0_OR_GREATER`. Looking at the build files our minimum TFM is, likely, `net9.0` hence this is unnecessary.

Considered for a bit adding an instruction that told copilot what the minimum TFM was but in the end decided that the preference would almost always be `#if NET` anyways hence just default to that.

Tested before / after locally this change resulted a better experience.
@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 18:33
@jaredpar
Copy link
Member Author

jaredpar commented Oct 3, 2025

@baronfel PTAL

Copy link
Contributor

github-actions bot commented Oct 3, 2025

This PR is targeting main, which is now for .NET 11-facing work. If you intended to target .NET 10, either retarget this PR to release/10.0.1xx or make sure you backport the change to release/10.0.1xx after merging. See #50394 for more details.

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 adds guidance for conditional compilation directives to improve the developer experience when using Copilot CLI for multi-targeting scenarios. The change addresses an issue where Copilot was defaulting to overly specific target framework conditionals like #if NET6_0_OR_GREATER when the simpler #if NET would be more appropriate for this codebase.

  • Added instruction to use #if NET for .NET Core specific code and #if NETFRAMEWORK for .NET Framework specific code

Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

LGTM. If we start publishing libraries from the SDK repo like we've been talking about then we'd need to adjust this. But that's a future concern.

@baronfel baronfel enabled auto-merge (squash) October 3, 2025 18:41
@baronfel baronfel merged commit f498215 into main Oct 3, 2025
29 checks passed
@baronfel baronfel deleted the dev/jaredpar/cpi-mt branch October 3, 2025 19:39
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.

2 participants