Suppress warnings about breaking changes to Newtonsoft.Json dependency in SignalR #9405
Conversation
natemcmaster
requested review from
analogrelay,
halter73,
mikaelm12 and
a team
as code owners
…pendency in SignalR
| @@ -27,4 +27,9 @@ | |||
| <Reference Include="System.Runtime.CompilerServices.Unsafe" /> | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'"> | |||
There was a problem hiding this comment.
This is technically a new package in 3.0.
We replaced the old Microsoft.AspNetCore.SignalR.Protocols.Json package with the Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson package and then removed the old one.
So this is now a new package and probably shouldn't have a baseline associated with it.
There was a problem hiding this comment.
Even so, it's the same package ID and the build system (and users) see this as a breaking change. It's okay since this was planned. This documents that intention in the code.
| @@ -0,0 +1,17 @@ | |||
| Build Errors | |||
| @@ -28,4 +28,9 @@ | |||
| <Reference Include="System.Threading.Channels" /> | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'"> | |||
| <!-- This dependency was replaced by Protocols.NewtonsoftJson between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. --> | |||
There was a problem hiding this comment.
Should we file an issue to do this after branching for 3.0? I guess it's not a big deal if we don't remove it right away.
There was a problem hiding this comment.
Well, such an issue is likely to get lost between now and RTM if it has to wait in the backlog. But, if you create a 4.0 milestone for it, that'll work.
There was a problem hiding this comment.
We don't need to file an issue, this will go away when we make System.Text.Json the default.
There was a problem hiding this comment.
It doesn't hurt if it stays around, but feel free to file an issue to clean up later. I'm writing these suppressions with the condition that checks version to makes sure the code doesn't become a problem later when we branch for 3.1 and beyond.
docs/BuildErrors.md
Outdated
|
|
||
| > error BUILD002: Package references changed since the last release... | ||
|
|
||
| Similar to BUILD001, but this error not suppressable. This error only appears in servicing builds which should not change references between assemblies or packages. |
There was a problem hiding this comment.
Minor but this will slow readers: "error is not…"
|
|
||
| ## Example: make a breaking change to references | ||
|
|
||
| If Microsoft.AspNetCore.Banana in 2.1 had a reference to `Microsoft.AspNetCore.Orange`, but in 3.0 this reference is changing to `Microsoft.AspNetCore.BetterThanOrange`, you would need to make these changes to the .csproj file |
There was a problem hiding this comment.
🍌 is already better than orange (🔶) ❕
| @@ -28,4 +28,9 @@ | |||
| <Reference Include="System.Threading.Channels" /> | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'"> | |||
| <!-- This dependency was replaced by Protocols.NewtonsoftJson between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. --> | |||
There was a problem hiding this comment.
Well, such an issue is likely to get lost between now and RTM if it has to wait in the backlog. But, if you create a 4.0 milestone for it, that'll work.
|
Hard to tell but the build failures in attempt 1 are likely flakiness. I started attempt 2. |
|
This comment was made automatically. If there is a problem contact ryanbrandenburg. I've triaged the above build. I've created/commented on the following issue(s) |
Resolves https://github.com/aspnet/AspNetCore-Internal/issues/1940
This adds support for listing a reference removal as an allowable breaking change.