EyalAr / lwip

Light Weight Image Processor for NodeJS
MIT License
2.37k stars 231 forks source link

Changes to make LWIP work on Node 10 #317

Closed kant2002 closed 2 years ago

kant2002 commented 6 years ago

This is list of changes which is required to make LWIP support Node 10. I fairly sure that this is also make it work on Node 8, but have to check that. There 2 changes here:

Update Zlib to 1.12.11

According to https://github.com/nodejs/node/commits/master/deps/zlib this is only version except 1.2.8 which was used so far, and this should fix installation on 8.X and 10.X versions of Node

I'm not sure is this proper fix for, or maybe better exclude Zlib from source code and rely on the Node-Gyp on that, that way just adding #if and setting different values PNG_ZLIB_VERNUM make it works.

Update source code to support latest NAN

Update code for LwipImage instantiation based on my understanding of NAN …

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 98.311% when pulling 47c5b084b7c81b2dee8c4740bed09d167b8bae8a on kant2002:node-10 into a559d24364139958ab99c9e153431bb408a031c6 on EyalAr:master.

kant2002 commented 6 years ago

@EyalAr I personally think that it is time to drop the plug from 0.10 and 0.12, 4.x is already not supported I think and 8.X is LTS. I could change support matrix for Travis. I don't get why AppVeyor is failing, maybe due to outdated dependencies, in that case it probably force drop support of 0.10, 0.12, but that's up to you. All I want is make project installable on Node 8.X and 10.X

EyalAr commented 6 years ago

@kant2002 thanks for this PR! Agree regarding dropping support for 0.10, 0.12 and 4.x. Could you please update the support matrix? Let's see if it also fixed the AppVeyor Build.

kant2002 commented 6 years ago

@EyalAr I feel particularly confused with AppVeyor, now it is failing and with another reason. Could you at least give feedback on the code itself, and I will take a look at AppVeyor meanwhile.

kant2002 commented 6 years ago

@EyalAr do you have any idea what could be done with Travis ?

kant2002 commented 6 years ago

@EyalAr could you restart build?

digitalica commented 5 years ago

for me lwip breaks on node 10 too. so this looks good... (dropping support below 4 makes sense I think)