-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Conversation
ICU can't "swap endianness" if its conversion, etc code is removed. We don't want this code in node, but it's needed for the tools. May affect 'target' inadvertently - to be analyzed. Basically this is nodejs#9046 but also blocks nodejs#8996
…CU data see nodejs#8996 Have not tested the resulting tarballs yet, posting for comment. Generates: - 9.8M node-v0.11.16-locales-be.tar.gz - 9.6M node-v0.11.16-locales-le.tar.gz
## Build data tarballs | ||
data-tarballs: $(LEDATATGZ) $(BEDATATGZ) | ||
|
||
$(LEDATATGZ) $(BEDATATGZ): node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on how node has been built, icupkg
may not have been built. Instead of depending on node
, maybe we could make these two targets depend on icupkg
? In the current state of the build system, an icupkg
target would depend on calling ./configure --with-intl=small-icu
first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icupkg isn't available in some configurations. maybe better to have this code run configure&&make
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it is used in the rule that builds these targets, so it must be available or am I missing something?
@srl295 Did you have some time to take a look at my comments? |
@misterdjules I thought the npm was the direction we wanted to go with this, as per #8996 |
@srl295 ... does this need to stay open? |
No it can close, this pr isn't the way forward. El sábado, 15 de agosto de 2015, James M Snell notifications@github.com
|
Awesome. Ok |
Work in progress on #8996
Generates: