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.

Sign up for GitHub

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

Jump to bottom

[wasm] add zone.tab to dotnet.timezones.blat #39731

Merged
merged 3 commits into from
Jul 22, 2020
Merged

[wasm] add zone.tab to dotnet.timezones.blat #39731

merged 3 commits into from
Jul 22, 2020

Conversation

tqiu8
Copy link
Contributor

@tqiu8 tqiu8 commented Jul 21, 2020

Fixes #39342 by adding zone.tab to dotnet.timezones.blat.

@tqiu8 tqiu8 requested a review from marek-safar as a code owner July 21, 2020 19:04
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@tqiu8 tqiu8 requested review from safern and lewing July 21, 2020 19:04
safern
safern previously approved these changes Jul 21, 2020
View reviewed changes
@safern safern dismissed their stale review July 21, 2020 19:10

Actually we need to renable the test

safern
safern reviewed Jul 21, 2020
View reviewed changes
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we re-enable the test that is disabled on: #39342 to actually make sure this fixes that?

@lewing
Copy link
Member

lewing commented Jul 21, 2020

The wasm test failure is #39473

@lewing lewing added the arch-wasm WebAssembly architecture label Jul 21, 2020
lewing
lewing approved these changes Jul 21, 2020
View reviewed changes
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good but adds some size, should we consider trimming the core list?

safern
safern approved these changes Jul 21, 2020
View reviewed changes
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module the size question.

@tqiu8 tqiu8 merged commit d77ef3e into dotnet:master Jul 22, 2020
@tqiu8 tqiu8 deleted the system-tz-fix branch July 22, 2020 19:41
Jacksondr5 pushed a commit to Jacksondr5/runtime that referenced this pull request Aug 10, 2020
@tqiu8 @Jacksondr5
[wasm] add zone.tab to dotnet.timezones.blat (dotnet#39731)
bd4905a 
* [wasm] add zone.tab to dotnet.timezones.blat to allow system time zones to load
* renable GetSystemTimeZones()
* reduce size of data file by removing unnecessary timezones
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lewing lewing lewing approved these changes

@safern safern safern approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

WASM: System.Tests.TimeZoneInfoTests.GetSystemTimeZones test fails in Browser
5 participants
@tqiu8 @Dotnet-GitSync-Bot @lewing @safern @karelz