-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Document System.Security.Cryptography.X509Certificates.X509Certificate2UI #2940
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
Conversation
mairaw
left a comment
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.
LGTM but this is a new API for 3.0. In .NET Framework, this class was static, so no constructor was shown. Now in .NET Platform Extensions 3.0 for example, it was changed to sealed and so now the class has a constructor.
|
One thing that I've noticed though is that the API catalog considers that constructor .NET Core 3.0 only, while docs is showing both: However, I can't find that API in the assemblies I have for .NET Core 3.0, just in the .NET Platform Extension one. What should be the correct behavior so I can open the bug? System.Windows.Extensions.dll was sent to me as part of extensions, not the shared framework. /cc @safern @terrajobst |
|
The build failed, but the root cause was unrelated to my change. Closing and reopening to trigger a new build. |
System.Windows.Extensions is not part of the shared framework, it ships as an OOB package, but I believe winforms brings it into its closure as a package reference, just as it does with System.Drawing.Common. The reason why it is showing in APICatalog is because we mixed up the the drops, but we will fix this sometime soon. |
|
@mairaw the build passed. Can we get this merged? |
|
@safern sounds good! @carlossanlop yes, merging now! |
This is not a new API, it was merely ported from Framework to Core: dotnet/corefx#31111
The only API missing documentation was the default constructor.
Adding @Anipik since he ported this class. Is the description of the default constructor good enough for you? It's not explicitly specified in the code you ported but it needs to get documented anyway.