-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Check for omissions in types supported by both Desktop and Core #19947
Comments
This isn't stricly blocking release but it's important to our compat goal so it really should happen. |
@safern could you grab this one also? You'll need to do a diff by some means or other, then run the list past folks like myself and Wes to figure which we do want in Core. |
Sure! Sounds good @danmosemsft |
@safern are you looking at this? |
@tarekgh not now, I'm currently focused on the netfx vertical tests. |
@danmosemsft this is marked as 2.0.0 so I think it is a priority. please have @safern knows which item should be done first. |
In progress |
Thanks @danmosemsft |
Cut table to the comment below // by Viktor |
This is a living post, which reflects the current status
CannotMakeMemberNonVirtual (non-virtual in the implementation but is virtual in the contract):
Classes are sealed and members are not virtual anymore. Change by design => https://github.com/dotnet/corefx/issues/17820 CannotMakeMoreVisible ('Family' in the implementation but 'FamilyOrAssembly' in the contract):
This is not a breaking change, ApiCompat was too noisy: dotnet/buildtools#1419 CannotMakeTypeAbstract (abstract in the implementation but is not abstract in the contract):
CannotRemoveBaseTypeOrInterface (does not implement interface in the implementation but it does in the contract):BaseType or Interface that doesn't exist in NS2.0:
All the _ interface were intentional removals and can be ignored. BaseType or Interface that does exist in NS2.0:
CannotSealType (sealed in the implementation but not sealed in the contract):
MembersMustExist (Member does not exist in the implementation but it does in the contract):
System.Data.*: https://github.com/dotnet/corefx/issues/17126 |
@danmosemsft @weshaggard Is it intentionally that SHA*Managed, HttpVersion and RuntimeEnvironment types are sealed/static in core and standard? It isn't restricted that much in netfx |
@davidsh to answer whether |
I think @stephentoub made the change to make it sealed. We should "un-seal" it if it is different from NETFX. |
I made that comment based on the list above/the baseline, but I'm confused now as I don't see it sealed in our code or NS20: |
See dotnet/standard#91 about why HttpVersion is static. I suspect the others are similar. @danmosemsft static implies abstract sealed type in metadata. |
@ViktorHofer are you still working on this one? just double checking the v2 issues are in progress. |
Yes, this is the parent for all the child issues. Some of them are now targeting the future milestone but for some of them I'm still awaiting answers from area owners therefore they are marked with 2.0. As soon as all 2.0 child issues are closed or changed to future I will close this one. |
I am going to clsoe this one since I think those child issues adequately track it and they are basically independent. |
I'm pleased with what we accomplished with this pass. |
Is there a workaround for |
Not that I'm aware of. cc @DnlHarvey, @atsushikan |
We have defined a common subset named .NET Standard 2.0 and analyzed (and fixed or baselined) any discrepancies between the Desktop and Core implementations of types in that subset.
We also care about any types that Desktop and Core both have that aren't in .NET Standard 2.0. If Core is missing some members that Desktop does have, this should be a conscious decision. Example of where it wasn't: dotnet/corefx#15238
One way to do this scan is go through https://raw.githubusercontent.com/dotnet/corefx/master/src/shims/ApiCompatBaseline.netfx461.txt, remove all the TypeMustExist lines (as these are types that Core doesn't have at all) and then manually remove all the lines relating to member exclusions we already decided to make as part of the Standard effort, as documented here https://github.com/dotnet/standard/tree/master/netstandard/ref
Anything we find we need to either add the API back or consciously decide to not do it.
Current status see below
The text was updated successfully, but these errors were encountered: