-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fixing the Facade's double typeforward to CultureTypes #6899
Conversation
@@ -5730,7 +5730,7 @@ public partial class CultureInfo : System.ICloneable, System.IFormatProvider | |||
public static System.Globalization.CultureInfo GetCultureInfo(string name) { throw null; } | |||
public static System.Globalization.CultureInfo GetCultureInfo(string name, string altName) { throw null; } | |||
public static System.Globalization.CultureInfo GetCultureInfoByIetfLanguageTag(string name) { throw null; } | |||
public static System.Globalization.CultureInfo[] GetCultures(System.Globalization.CultureTypes types) { throw null; } |
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.
could you add a small comment telling why we are commenting this API and the enum? and we need to open issue tack understanding what is going on.
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.
Sure, I'll let the builds finish first to make sure my CI is green, and then I'll add the comment and merge
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.
No need anymore, found the right fix.
LGTM. |
Actually I just figured out what the root cause is and how to get an easy fix, I'll update this PR now. @tarekgh with this new fix, CultureTypes will be public again. |
c151112
to
740db98
Compare
@weshaggard do we know why we have these type forward for the following?
|
@mmitche looks like my Linux ARM Emulator failures are due to "No space left on device" issue: #6676 |
@joperezr tracked with #6676 |
LGTM. Please do verify that we don't have any other similar issues. Can you verify that none of the other types in this file are duplicated? |
@joperezr would be good if you can quickly edit the title and the description of this issue. just in case if we reference it in the future :-) |
When merging #6694 CultureTypes was moved from internal to public because it was needed by one of the methods of CultureInfo. CultureTypes was already being type-forwarded in the facade project before GenFacades even ran, so after GenFacades this was causing the facade to have two different typeforwards to CultureTypes. Because of this, Microsoft.CCI was basically off by one in the type table ref, which was causing all nested types to have the wrong forwarder.
cc: @weshaggard @danmosemsft @stephentoub
FYI: @hann013 @sepidehms @tarekgh