Skip to content

Adding guidance on what is considered a breaking change in the .NET SDK #45288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 20, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions documentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Documents Index
- [Roadmap and OS support](https://github.com/dotnet/core/blob/main/roadmap.md)
- [Comprehensive CLI documentation](https://docs.microsoft.com/en-us/dotnet/articles/core/preview3/tools/)
- [ASP.NET Core Command Line Tools](general/aspnetcore-tools.md)
- [Guidelines for how to introduce a new diagnostic or breaking change](project-docs/breaking-change-guidelines.md)

## Working with the CLI repo

Expand Down
81 changes: 81 additions & 0 deletions documentation/project-docs/breaking-change-guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# SDK Breaking Change and Diagnostic Guidelines

This document provides guidelines for introducing new diagnostics or breaking changes to the .NET SDK, which configuration knobs are available, and what criteria should guide the decision around severity, timeline for introduction, and deprecation of diagnostics.

## Overall procedure

In general, we want to make updating the .NET SDK as smooth as possible for developers. This means:

* introducing new changes in a staged/gradual way
* trying to tie opinionated analyzers/diagnostics to a mechanism that requires explicit user opt-in
Copy link
Member

Choose a reason for hiding this comment

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

not sure what opinionated means in this context. Maybe just remove or are there specific types of diagnostics that you would include and some you wouldn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

* providing a way to opt out of a change entirely

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The other bulleted lists begin with a capital letter. I prefer capital for lists and would suggest doing that here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Changed.

## Kinds of changes

There are many kinds of breaking changes that can ship in the .NET SDK, such as:

* * New MSBuild warnings and errors (props/targets)
* New NuGet warnings and errors.
* For example, NuGet Audit.
* Roslyn Analyzers and CodeFixes
* This includes trimming/ILLink analyzers and codefixes
* Behavioral/implementation changes
* MSBuild engine changes like MSBuild Server
* Implementation changes for MSBuild Tasks
* NuGet Restore algorithm enhancements
* Changes to DotNet CLI grammar
* Changes to defaults in CLI flags that impact behavior
Copy link
Member

Choose a reason for hiding this comment

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

changes to CLI behavior itself. For example, we're considering changing dotnet clean to clean everything, not just your tfm/rid config which is what it does today.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added.

* For example, `--configuration` flag defaulting to `Release` instead of `Debug`

## Configuration Knobs

The following knobs are available to enable/disable these changes (some may not apply to all kinds of changes):
Copy link
Member

Choose a reason for hiding this comment

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

Change waves and LangVer also apply to the tooling ecosystem.

Copy link
Contributor

Choose a reason for hiding this comment

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

LangVer - I read https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/configure-language-version, and it looks like it's the same as TFM for .NET Core. So, I'm wondering in which cases langver would be used instead of TFM.

Change waves - I found a description here: https://learn.microsoft.com/en-us/visualstudio/msbuild/change-waves?view=vs-2022. Moving forward, do we recommend people using change waves or using the new SdkAnalysisLevel property?

@baronfel

Copy link
Member Author

Choose a reason for hiding this comment

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

LangVersion is often the same as the numeric part of TFM, but that's purely a convenience. Users often set it explicitly when

  • multitargeting and wanting to ensure a consistent level of analysis across TFMs
  • when they want to use preview language features

Copy link
Member Author

Choose a reason for hiding this comment

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

MSBuild Change Waves are necessary still for changes to MSBuild engine behavior, especially changes that occur before project evaluation takes place (e.g. globbing). MSBuild engine changes remain a place where we do not have great user-facing controls. MSBuild logic (props/targets files) are great for SdkAnalysisLevel, WarningLevel, etc knobs that we already have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.


* [TargetFramework](https://learn.microsoft.com/en-us/dotnet/standard/frameworks)
* [SdkAnalysisLevel](https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#sdkanalysislevel)
* EditorConfig (for Analyzers)
* AnalysisLevel (for Analyzers/CodeFixes)
* WarningLevel

## Deciding how to Surface Changes

* Implement changes in an informational/non-blocking way initially

What this means will vary change-to-change. For example, for a change expressed as an Analyzer or MSBuild diagnostic, consider
Informational level severities initially. For a behavioral change on a CLI, consider an informational message written to the
Copy link
Member

Choose a reason for hiding this comment

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

theoretically, this is also breaking but we have to draw a line somewhere and I think if some script is reading and depending on the stderr output, that seems like a reasonable line to draw.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rainersigwald - Based on your experience, should changes int the output be considered breaking?

Copy link
Member

Choose a reason for hiding this comment

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

Changes in output can certainly be breaking. MSBuild has had to demote some high-importance info messages (e.g. dotnet/msbuild#9228) because including location and/or a code made them look "too much" like warnings for enough users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed.

stderr channel on the console instead of making the stdout output un-parseable by tools.

Concrete example: When the `dotnet new --list` command was changed to `dotnet new list` to adhere to CLI design guidelines,
the old command was still the default supported and a warning was written to the console when the old form was used pointing users to the new form. This allowed users and scripts to continue using the old form and gradually migrate to the new.

* Gradually increase severity over each release

If a change in introduced in an informational/non-blocking way, determine the time frame where it is safe to increase the severity. For Analyzers, this may mean tying it to the next value of AnalysisLevel (which is downstream of TFM). For small MSBuild and NuGet diagnostics, this may mean tying it to the next Warning Level or SdkAnalysisLevel. For CLI changes, this may mean tying it to the next LTS major version of the SDK. Ideally the way you would structure this increase would be automated and documented so that users know what's coming down the pipe.

* Cut-over to new behavior after a long introduction period

After the change has been introduced in a gradual way, cut over to the new behavior. This may mean removing the old behavior entirely, or it may mean making the new behavior the default and providing a way to opt out of it. It is important that you
provide enough time for users to adapt - for example the `dotnet new --list` example above took an entire major release to make the new forms the default.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest a newline for each start of sentence to make future updates cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm following the convention of other markdown in the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Git diffs on markdown work better with my suggestion. Up to you whether that's important.


* Always provide a way to opt out of the change

Have some kind of knob that allows users to opt out of the change entirely. This could be a flag, an environment variable, or a global.json setting. This allows users to continue using the old behavior if they need to in exceptional situations. It is
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to avoid global.json settings. That should be used in limited scenarios and not for flags like this. I also think that env variables are not preferred if you can help it as you have to set them before you run (though they are required for some types of changes like msbuild enginge changes). By flag, you should call out project property (unless you meant something else).

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. Fixed.

important to document this knob and its behavior in the SDK documentation. It is also important to define a timeline for when this knob will be removed entirely, forcing users to adopt the new behavior.
Copy link
Member

Choose a reason for hiding this comment

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

fwiw, I might never remove some knobs if they are simple properties or env variables. I don't think Roslyn removes langver support but we should check with them.

Copy link
Member

Choose a reason for hiding this comment

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

ahh, you mention that may be never for analyzers below. I might include items that have a simple on/off.

Copy link
Contributor

Choose a reason for hiding this comment

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

I removed the break line.


For systems like Analyzers that time may be 'never', because the cost of detection is so low. This is a product-level decision that is hard to give universal guidance for.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe italics instead of a code callout for never

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.


* Hook into the unified SdkAnalysisLevel knob to allow users to easily opt out of all changes

This knob exists so that users can safely and consistently say "for whatever reason, I just need you to act like SDK version X". This is the one-stop shop - users no longer need to know about all the individual knobs that are available to them.

* Tie potentially impactful changes to the TFM targeted by the application/library.

Changes that are expected to cause significant disruption should only be introduced behind the Target Framework knob. This eliminates business continuity and allows developers to address changes needed as part of scheduled work to migrate a codebase to a new TFM.
Copy link
Member

Choose a reason for hiding this comment

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

I might explicitly state that this is preferred where possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current wording is a bit stronger, and I think that's something good. Thoughts?


Concrete example: NuGet warnings for vulnerable transitive dependencies were introduced only for applications targeting .NET 10 and higher.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the terminology "concrete" is a bit strong and unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will change to "specific"


## Other recommendations
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to mention the breaking change label, tactics, or any of the internal aliases?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is public documentation, but the breaking-change label is relevant. I'll add that.


* If possible, introduce significant breaking changes in a non-LTS release.
Copy link
Member

Choose a reason for hiding this comment

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

fwiw, .NET leadership does not consider there to be a separate bar for LTS versus STS when it comes to things like this. Not sure if you included this coming from VS guidance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed.

* Publish blog posts and update public documentation as appropriate as early as possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

Who is the audience for this document? Who are we asking to publish a blog? I would imagine this is an internal PM responsibility, but it reads to me like we are asking anyone contributing to the SDK to publish a blog.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

* Consider creating and pinning an issue in the corresponding GitHub repository where customers can provide feedback.