Nullable: all calendars #23469
Nullable: all calendars #23469
Conversation
| @@ -54,9 +55,9 @@ private static string GetJapaneseEnglishEraName(int era) | |||
| return era <= s_JapaneseErasEnglishNames.Length ? s_JapaneseErasEnglishNames[era - 1] : " "; | |||
| } | |||
|
|
|||
| private static bool GetJapaneseEraInfo(int era, out DateTimeOffset dateOffset, out string eraName, out string abbreviatedEraName) | |||
| private static bool GetJapaneseEraInfo(int era, out DateTimeOffset dateOffset, out string? eraName, out string? abbreviatedEraName) | |||
There was a problem hiding this comment.
I'm not 100% sure about this ones.
@tarekgh do you know if these out string parameters can be null?
There was a problem hiding this comment.
The method will return false if we couldn't get non null strings.
There was a problem hiding this comment.
@krwq what happen if one of these strings is null?
There was a problem hiding this comment.
assuming we return false on failure we are good - if API returns true AND null as one of the strings then it will be populated to EraInfo constructor
There was a problem hiding this comment.
Then those 2 values in the call site should be nullable. I guess that could break the corert build
There was a problem hiding this comment.
actually this is my bad - forgot the private CI doesn't build core rt
There was a problem hiding this comment.
Oh I thought you actually built corert with this changes locally. Might be worth doing it just to make sure.
There was a problem hiding this comment.
It is not trivial. I don’t think it is worth spending a day on adding it. If we see it starts causing pain then we can invest in it.
There was a problem hiding this comment.
im not sure how to build corert locally as well - we can fix any errors on the main PR since we have CI in there - there is very little CoreRT changes
There was a problem hiding this comment.
Rather than one question. It looks good to me.
cc: @stephentoub
* calendar * nullable: all calendars * fix likely corert error
* calendar * nullable: all calendars * fix likely corert error
* calendar * nullable: all calendars * fix likely corert error
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Signed-off-by: dotnet-bot <[email protected]>
* calendar * nullable: all calendars * fix likely corert error Commit migrated from dotnet/coreclr@bbcfd3a
* calendar * nullable: all calendars * fix likely corert error Commit migrated from dotnet/coreclr@90f5615
No description provided.