Fix Hebrew calendar month names list #16247
Conversation
|
Do we need a regression test in CoreFX for this ? |
|
test Windows_NT x64_arm64_altjit Checked corefx_baseline |
Yes, I have the test ready but I didn't open the PR yet. if you want I can open the PR from now. |
| @@ -257,6 +267,15 @@ private static bool EnumMonthNames(string localeName, CalendarId calendarId, Cal | |||
| callbackContext.Results.Add(string.Empty); | |||
| } | |||
|
|
|||
| if (callbackContext.Results.Count > 13) | |||
| { | |||
There was a problem hiding this comment.
Add Debug.Assert(callbackContext.Results.Count == 14 && calendarId == CalendarId.HEBREW) ?
There was a problem hiding this comment.
In the future we can get calendars which can have 13 months too. so I prefer not putting this assert.
There was a problem hiding this comment.
This path is only taken for 14 months or more...
I was just wondering whether assert like that would help us if some other calendar needs the special treatment like Hebrew.
There was a problem hiding this comment.
I'll add the assert. I think it make sense to be conscious about any other similar behaving calendar
|
The failures in Windows_NT x64_arm64_altjit Checked corefx_baseline are because the API compat complaining about the new System.Runtime.Intrinsics API. so these are not related. |
|
It also means that no corefx tests run on your change... |
That is fine. I have run the corefx globalization tests against my private manually. I just added these legs to get extra confidence |
* Fix Hebrew calendar month names list Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
* Fix Hebrew calendar month names list Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
* Fix Hebrew calendar month names list Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
* Fix Hebrew calendar month names list Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
* Fix Hebrew calendar month names list Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
* Fix Hebrew calendar month names list Commit migrated from dotnet/coreclr@dc4266c
Fixes #16203