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

tools: update tzdata to 2019c #30356

Closed
wants to merge 1 commit into from
Closed

Conversation

albertyw
Copy link
Contributor

Based on the instructions in #30211 (comment), I updated the time zone database separately from the rest of the ICU data. This should be a cut-down version of #30232 .

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@Trott
Copy link
Member

Trott commented Nov 10, 2019

@srl295 @MylesBorins @nodejs/intl

@Trott Trott added the i18n-api Issues and PRs related to the i18n implementation. label Nov 10, 2019
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

LGTM

@MylesBorins
Copy link
Contributor

I would like us to fastrack this

@MylesBorins MylesBorins added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 12, 2019
@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 12, 2019
@srl295
Copy link
Member

srl295 commented Nov 12, 2019

note also #29540

@Trott
Copy link
Member

Trott commented Nov 12, 2019

This needs one more approval before it can land. @MylesBorins ?

@srl295
Copy link
Member

srl295 commented Nov 12, 2019

So this could be a periodic chore.

Related: I don't think we want to go down the route where npm install full-icu-tz becomes the new npm install full-icu do we?

@richardlau
Copy link
Member

So this could be a periodic chore.

Related: I don't think we want to go down the route where npm install full-icu-tz becomes the new npm install full-icu do we?

That depends on how often timezone data changes and whether we want to enable a pathway where users can apply timezone fixes without having to update Node.js (thinking more of the LTS lines here where releases are more spread out, unless we need to class timezone updates as more urgent requiring out-of-schedule releases).

cc @nodejs/lts

@albertyw
Copy link
Contributor Author

tzdata changes pretty often. For example, 2018 saw 9 tzdata updates. 2019 so far has had 3. For node to stay up to date, a diff like this should happen every few months. (it just so happens that most changes are relatively minor, like for Fiji or North Korea; the current push is because a much larger country Brazil decided to change its time zone policy).

Ref: https://data.iana.org/time-zones/tzdb/NEWS

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

Rubber stamp

@targos targos removed the fast-track PRs that do not need to wait for 48 hours to land. label Nov 13, 2019
targos pushed a commit that referenced this pull request Nov 13, 2019
Fixes: #30211
PR-URL: #30356
Reviewed-By: Steven R Loomis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos
Copy link
Member

targos commented Nov 13, 2019

Landed in 31ea341

@targos targos closed this Nov 13, 2019
@targos
Copy link
Member

targos commented Nov 13, 2019

@MylesBorins @srl295 If we want to land it on v12.x, we need a backport PR because the data file is different there (not compressed, small-icu).

MylesBorins added a commit to MylesBorins/node that referenced this pull request Nov 13, 2019
MylesBorins added a commit to MylesBorins/node that referenced this pull request Nov 13, 2019
targos pushed a commit that referenced this pull request Nov 14, 2019
Refs: #30211
Refs: #30356

PR-URL: #30478
Reviewed-By: Steven R Loomis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@albertyw albertyw deleted the update-tz branch November 14, 2019 19:21
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
Fixes: #30211
PR-URL: #30356
Reviewed-By: Steven R Loomis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Nov 19, 2019
BethGriggs pushed a commit that referenced this pull request Dec 3, 2019
Refs: #30211
Refs: #30356

PR-URL: #30479
Reviewed-By: Steven R Loomis <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
MylesBorins added a commit that referenced this pull request Dec 17, 2019
Refs: #30211
Refs: #30356

PR-URL: #30479
Reviewed-By: Steven R Loomis <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
albertyw added a commit to albertyw/node-1 that referenced this pull request Jan 21, 2020
Update the version of the bundled ICU (deps/icu-small) to ICU version
65.2.

Fixes: nodejs#30211
Fixes: nodejs#29540
Backport-PR-URL: nodejs#31434
PR-URL: nodejs#30356
PR-URL: nodejs#30232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. i18n-api Issues and PRs related to the i18n implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants