-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Api-diff between 7.0-preview7 and 7.0-rc1 #7808
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API changes in my areas LGTM. CC. @dakersnar
Mostly just the nullability annotation fixes
release-notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.ComponentModel.md
Outdated
Show resolved
Hide resolved
@KlausLoeffelmann can you sanity check the WinForms changes you made here? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New WinForms CommandBinding and DataContext APIs look OK.
release-notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.IO.md
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Windows.Forms.* look good 👍
@KlausLoeffelmann @dreddy-work to confirm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Data looks good.
...w/api-diff/rc1/Microsoft.AspNetCore.App/7.0-rc1_Microsoft.AspNetCore.Authorization.Policy.md
Outdated
Show resolved
Hide resolved
...w/api-diff/rc1/Microsoft.AspNetCore.App/7.0-rc1_Microsoft.AspNetCore.Connections.Features.md
Outdated
Show resolved
Hide resolved
...es/7.0/preview/api-diff/rc1/Microsoft.AspNetCore.App/7.0-rc1_Microsoft.AspNetCore.Routing.md
Outdated
Show resolved
Hide resolved
System.Runtime.Versioning, System.Reflection, System.Reflection.Emit |
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
- public HMACMD5(); | ||
+ [UnsupportedOSPlatformAttribute("browser")] | ||
+ public HMACMD5(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods shouldn't show as deleted.
- public HMACMD5(); | |
+ [UnsupportedOSPlatformAttribute("browser")] | |
+ public HMACMD5(); | |
+ [UnsupportedOSPlatformAttribute("browser")] | |
public HMACMD5(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not showing up as deleted. AsmDiff shows the whole API as diffed when a new attribute decorates it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as with classes dotnet/arcade#10982
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not showing up as deleted.
On net, it isn't, no. But to a casual interpretation by a human, it is, since there's a -
line for it.
The content of System.Security.* and System.Formats.Asn1 looks correct to me; but the attribute-only changes are showing as very noisy diffs, suggesting we removed a lot of methods. Those diffs really need to be cleaned up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Text.Json & System.Linq changes LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Text.Json LGTM
...iew/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Runtime.Serialization.DataContracts.md
Outdated
Show resolved
Hide resolved
...iew/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Runtime.Serialization.DataContracts.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.Diagnostics LGTM
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
...notes/7.0/preview/api-diff/rc1/Microsoft.NETCore.App/7.0-rc1_System.Security.Cryptography.md
Outdated
Show resolved
Hide resolved
@bartonjs I can apply your suggestions for APIs that only had their attributes changed, but note that this is something that affects many APIs and has been happening with AsmDiff for a long time. I really don't think this is worthy of this much attention and the priority seems low. |
Co-authored-by: Jeremy Barton <[email protected]>
Co-authored-by: Jeremy Barton <[email protected]>
If you see unusual diffs in this PR, please help fixing them via suggestions. I would greatly appreciate it.
ASP.NET: https://github.com/orgs/dotnet/teams/aspnet-api-review
WinForms: @merriemcgaw @RussKie @JeremyKuhne
WPF: @singhashish-wpf and @pchaurasia14
Libraries: @danmoseley @jeffhandley @karelz @ericstj @stephentoub
@dotnet/area-microsoft-win32
@dotnet/area-system-buffers
@dotnet/area-system-componentmodel
System.Data @ajcvickers @DavoudEshtehari @David-Engel
System.Diagnostics @tommcdon
@dotnet/area-system-formats-asn1
@dotnet/area-system-io
@dotnet/area-system-linq
System @dotnet/area-system-runtime
System.Net* @dotnet/ncl
@dotnet/area-system-numerics
@dotnet/area-system-reflection-emit
@dotnet/area-system-reflection
System.Runtime.InteropServices @dotnet/interop-contrib
@dotnet/area-system-runtime
System.Runtime.Serialization @HongGit @StephenMolloy
System.Runtime.Versioning @buyaa-n