Skip to content
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.

Sign up for GitHub

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

Jump to bottom

API diff between .NET 10 Preview1 and .NET 10 Preview2 #9771

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

API diff between .NET 10 Preview1 and .NET 10 Preview2 #9771

wants to merge 5 commits into from
+573 −0

Conversation

carlossanlop
Copy link
Member

@carlossanlop carlossanlop commented Feb 27, 2025 •
edited
Loading

Repo area owners:

Libraries area owners:

  • System.Collections @dotnet/area-system-collections
  • System.Linq @dotnet/area-system-linq
  • System.Numerics @dotnet/area-system-numerics
  • System.Reflection @dotnet/area-system-reflection-metadata
  • System.Runtime, System.Runtime.CompilerServices, System.Runtime.InteropServices @dotnet/area-system-runtime @dotnet/area-system-runtime-compilerservices @dotnet/interop-contrib

New tool to generate diffs!

I have an open PR with the new tool we'll be using from now on to generate these API diffs: dotnet/sdk#46425

If you see any bugs, or if you don't see any APIs that should've showed up, please let me know.

Main differences with the old tool:

  • It uses Roslyn instead of Cci.
  • It has unit tests.
  • The files are being generated by assembly, not by namespace. So you might find System.Runtime.CompilerServices under the System.Runtime file.
  • Non-void returning methods show a throw null. Void returning methods show {} instead of just the ;.
  • Events will show their add and remove accessors.

Bugs (currently working on them):

  • Some attributes like EditorBrowsable should've been ignored, but sometimes they show up.
  • Attributes in method arguments are not getting included.
  • Private members are supposed to be ignored, yet private int _dummyPrimitive is showing up in many places.
  • The System. prefix in API fullnames is unexpectedly getting removed in some cases, showing the diff as having an API added.

@carlossanlop carlossanlop self-assigned this Feb 27, 2025
@carlossanlop carlossanlop mentioned this pull request Feb 27, 2025
Draft
Tanya-Solyanik
Tanya-Solyanik reviewed Feb 27, 2025
View reviewed changes
.../preview2/api-diff/Microsoft.WindowsDesktop.App/10.0-preview2_System.Windows.Forms.Design.md
public sealed class Options
{
- public System.Reflection.StrongNameKeyPair? keyPair;
+ public Reflection.StrongNameKeyPair? keyPair;
Copy link
Member

Choose a reason for hiding this comment

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

This is not really a change, the preview2 version can detect that this class's namespace starts with System and preview1 does not.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah -- omitting System is confusing here. Can we adjust the tool so that it doesn't do this?

Tanya-Solyanik
Tanya-Solyanik reviewed Feb 27, 2025
View reviewed changes
.../preview2/api-diff/Microsoft.WindowsDesktop.App/10.0-preview2_System.Windows.Forms.Design.md
+ protected virtual System.Type? GetType(ComponentModel.Design.IDesignerHost? host, Reflection.AssemblyName? assemblyName, System.String typeName, System.Boolean? reference) { throw null; }
}
}
namespace System.Windows.Forms.Design
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 had added some classes to this namespace, I need to double check that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. Let me know.

@ericstj
Copy link
Member

ericstj commented Feb 28, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericstj ericstj ericstj left review comments

@Tanya-Solyanik Tanya-Solyanik Tanya-Solyanik left review comments

@rbhanda rbhanda Awaiting requested review from rbhanda rbhanda is a code owner

@leecow leecow Awaiting requested review from leecow leecow is a code owner

@victorisr victorisr Awaiting requested review from victorisr victorisr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@carlossanlop carlossanlop

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@carlossanlop @ericstj @Tanya-Solyanik