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

deps: icu: apply workaround patch to 62.1 for significant digits #23764

Merged
merged 0 commits into from
Oct 23, 2018

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 19, 2018

patch from https://chromium.googlesource.com/chromium/deps/icu/+/master/patches/nf_maxsig.patch

from https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU bug: https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

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

@srl295 srl295 self-assigned this Oct 19, 2018
@nodejs-github-bot nodejs-github-bot added i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory. labels Oct 19, 2018
@srl295 srl295 changed the title deps: icu: apply workaround patch deps: icu: apply workaround patch to 62.1 for significant digits Oct 19, 2018
@srl295
Copy link
Member Author

srl295 commented Oct 19, 2018

for ICU 62 this warning will appear:

WARNING: Using floating patch "tools/icu/patches/62/source/i18n/decimfmt.cpp" from "tools/icu"
WARNING: warnings were emitted in the configure phase

This will go away with ICU 63, #23715 because ICU 63 has fixed this issue.

@srl295 srl295 force-pushed the patch-icu-62-sigdig branch from 4b61932 to 6576f41 Compare October 19, 2018 22:58
@srl295
Copy link
Member Author

srl295 commented Oct 19, 2018

@jasnell thanks. Rebased and fixed a linter problem (long line)

@srl295 srl295 force-pushed the patch-icu-62-sigdig branch from 6576f41 to 4a21d69 Compare October 22, 2018 22:19
@srl295 srl295 requested a review from refack October 23, 2018 01:27
@srl295
Copy link
Member Author

srl295 commented Oct 23, 2018

@srl295 srl295 closed this Oct 23, 2018
@srl295 srl295 force-pushed the patch-icu-62-sigdig branch from 4a21d69 to 4194b05 Compare October 23, 2018 20:33
@srl295 srl295 merged commit 4194b05 into nodejs:master Oct 23, 2018
@srl295 srl295 deleted the patch-icu-62-sigdig branch October 23, 2018 20:33
targos pushed a commit that referenced this pull request Oct 24, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins
Copy link
Contributor

@srl295 I've landed this on 8.x and 10.x staging. Please lmk if it shouldn't have landed

MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 26, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@codebytere codebytere mentioned this pull request Nov 27, 2018
rvagg pushed a commit that referenced this pull request Nov 28, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
ICU 62.1 had a bug where certain orders of operations would not
work with the minimum significant digit setting. Fixed in
ICU 63.1. Applied the following patch from v8.

https://chromium-review.googlesource.com/c/chromium/deps/icu/+/1128503

ICU Bug:
https://unicode-org.atlassian.net/browse/ICU-20063

Fixes: #22156

PR-URL: #23764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@codebytere codebytere mentioned this pull request Nov 29, 2018
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants