Skip to content
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

Remove TimeZoneConverter dependency on net6.0 #398

Closed
wants to merge 8 commits into from
Closed

Conversation

sebastienros
Copy link
Owner

No description provided.

Fluid/Filters/MiscFilters.cs Outdated Show resolved Hide resolved
@sebastienros
Copy link
Owner Author

Can someone help me understand why the test is failing on windows for GH? It works locally for me, on Windows 11. The call to FindSystemTimeZoneById throws an exception for "America/Los_Angeles" but it's a valid IANA name.

@hishamco
Copy link
Collaborator

I will try to test it in my Windows

@hishamco
Copy link
Collaborator

Same here, still investigating on this ...

@hishamco
Copy link
Collaborator

I tried Pacific Standard Time the exception is gone, but still some unit tests are failing

@sebastienros
Copy link
Owner Author

Pacific Standard Time is a standard windows TZ name. The tests here try to use the IANA names, which is supported on Windows only since dotnet 6. It's like the test is not actually using dotnet 6 :/

@sebastienros
Copy link
Owner Author

@lahma
Copy link
Collaborator

lahma commented Nov 19, 2021

Might be worthwhile to check how it behaves on newer windows image, like 2022 preview: https://github.blog/changelog/2021-08-23-github-actions-windows-server-2022-with-visual-studio-2022-is-now-available-on-github-hosted-runners-public-beta/ . The windows-latest is Windows Server 2019.

@sebastienros
Copy link
Owner Author

Might be related to dotnet/runtime#60175

@sebastienros
Copy link
Owner Author

sebastienros commented Nov 19, 2021

The logs seem to indicate Windows 10 October 2018 Update (Microsoft Windows 10.0.17763).
But anyhow since the behavior will depend on some OS settings, I won't be able to make it work. I can only force ICU when referencing the package like this:

<PackageReference Include="Microsoft.ICU.ICU4C.Runtime" Version="68.2.0.9" />
<RuntimeHostConfigurationOption Include="System.Globalization.AppLocalIcu" Value="68.2" />

And I am not even sure that's something that can be done from a package. So for now we'll have to keep the dependency on TimeZoneConverter.

@sebastienros sebastienros deleted the sebros/tz branch November 19, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants