-
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
[Apple-Mobile][Globalization] Refactoring of CalendarData.iOS and new DateTimeFormatInfo* tests #102464
[Apple-Mobile][Globalization] Refactoring of CalendarData.iOS and new DateTimeFormatInfo* tests #102464
Conversation
Tagging subscribers to this area: @dotnet/area-system-globalization |
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.Icu.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.iOS.cs
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.
Looks good to me. Thanks for cleaning it up!
...tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoAbbreviatedDayNames.cs
Outdated
Show resolved
Hide resolved
...tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoAbbreviatedDayNames.cs
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.
I have added minor comments. Please ensure to apply that on all added tests. LGTM otherwise.
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
The extra-platform failures are known and are tracked at: #103472. The |
Description
This PR has 3 components:
1. Refactoring of
CalendarData.iOS.cs
The goal was to unify logic with
CalendarData.ICU
. Eventually it should be possible to fully merge these two together.2. Fix of
DateTimeFormatInfo.LongDatePattern
API:The previously returned format was incorrect. Previously we were returning the
NSDateFormatterLongStyle
. However, for .NET theLongDatePattern
(https://learn.microsoft.com/en-us/dotnet/api/system.globalization.datetimeformatinfo.longdatepattern?view=net-8.0) corresponds toNSDateFormatterFullStyle
(https://developer.apple.com/documentation/foundation/nsdateformatterstyle/nsdateformatterfullstyle).3. Adding new test cases for
DateTimeFormatInfo*
APIsMany of
DateTimeFormatInfo*
APIs were only tested inInvartianGlobalization
settings before. The following tests were added for platforms with ICU globalization:DateTimeFormatInfoAbbreviatedDayNames
(en-US
,fr-FR
)DateTimeFormatInfoAbbreviatedMonthGenitiveNames
(en-US
,fr-FR
)DateTimeFormatInfoAbbreviatedMonthNames
(en-US
,fr-FR
)DateTimeFormatInfoDayNames
(en-US
,fr-FR
)DateTimeFormatInfoLongDatePattern
(en-US
,fr-FR
)DateTimeFormatInfoMonthDayPattern
(en-US
,fr-FR
)DateTimeFormatInfoMonthGenitiveNames
(en-US
,fr-FR
)DateTimeFormatInfoMonthNames
(en-US
,fr-FR
)The
en-US
,fr-FR
locales were selected because they are generally the most stable between ICU versions.Contributes to #100240