Uh oh!
There was an error while loading. Please reload this page.
deps: Intl: ICU 57 bump - #6064
Conversation
srl295
commented
Apr 5, 2016
srl295
commented
Apr 5, 2016
OSX: passes except |
There was a problem hiding this comment.
thanks, refactor and removed the todo. (it's done now that there are 57 AND 55 clauses)
* bump to ICU 57.1 - update URL / hash * add exclusion list for 57 (sorry, missed 56). This will reduce the binary footprint on some platforms. * Exclude cstr.cpp to work around http://bugs.icu-project.org/trac/ticket/12451 on Windows/MSVC Fixes: nodejs#6058
jasnell
commented
Apr 7, 2016
LGTM if CI is green |
jasnell
commented
Apr 7, 2016
@srl295 ... would you see any problem in getting this back to v4 also? |
MylesBorins
commented
Apr 7, 2016
ci: https://ci.nodejs.org/job/node-test-pull-request/2207/ If we see the same failures we are seeing on #6088 we might want to consider putting #6088 on ice until the problems are worked out in this thread (to minimize churn and checked in assets) |
MylesBorins
commented
Apr 7, 2016
LGTM if CI is green |
srl295
commented
Apr 7, 2016
@jasnell no problem in getting this back to v4 @thealphanerdheadsmack that's what happened. I didn't merge badly, I just didn't reapply the changes here to Anyways looks like CI here is green except for some unrelated arm tests. |
jbergstroem
commented
Apr 7, 2016
@srl295: Are icu versions backwards-compatible -- I mean, do you have to inspect the changelog for every release or is there some kind of versioning you can rely on? |
srl295
commented
Apr 8, 2016
@jbergstroem ICU is source-level backwards compatible. So something that compiled against 56 should be able to compile against 57, etc. |
| # alphabetic index | ||
| '../../deps/icu/source/i18n/alphaindex.cpp', | ||
| # BOCSU |
There was a problem hiding this comment.
Looks like this should be '## Strip out the following for ICU 57 only.'
srl295
commented
Apr 8, 2016
jasnell
commented
Apr 8, 2016
+1.. perhaps keep the commits separate, however. |
srl295
commented
Apr 8, 2016
@jasnell ok, good idea. I'll implement this ticket in a separate commit |
srl295
commented
Apr 9, 2016
OK the version bump (url/hash) is now part of #6088 |
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included? ( No )
existing APIs, or introduces new ones)? ( n/a )
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Affected core subsystem(s)
Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)
Description of change
Please provide a description of the change here.
This will reduce the binary footprint on some platforms.
http://bugs.icu-project.org/trac/ticket/12451 on Windows/MSVC
Fixes: #6058