ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 208 forks source link

Lock 'Request' package to 2.81.0. #314

Closed dmaltsiniotis closed 6 years ago

dmaltsiniotis commented 6 years ago

The 'Request' package maintainers have made a breaking change in version 2.82.0+ that no longer works on Node 0.10.x or 0.12.x. Since this package still specifies a supported engine of node 0.10.0, this PR locks the 'Request' package version to 2.81.0 forever.

While the "correct" solution here may be to not use node 0.x anymore since it's been deprecated, there are some packages and configurations that will break with change made in Request.

dmaltsiniotis commented 6 years ago

See this issue for more information: https://github.com/request/request/issues/2774

hypesystem commented 6 years ago

This makes sense to me.

Updating supported node version looks like a breaking change to me, so we should do that at some later point. Thoughts, @eladnava ?

eladnava commented 6 years ago

Hi @hypesystem and @dmaltsiniotis, I would say, the best solution would be to lock down node-gcm to the last version of request that supports Node 0.x. There is no need to introduce any breaking change.

What do you think?

dmaltsiniotis commented 6 years ago

@eladnava @hypesystem I think we're all on the same page here. As it stands today, the node-gcm package does not work when attempting to use the latest version on node 0.x. This PR locks the version of Request down to the last one that worked on node 0.x, which was 2.81.0. I don't think this introduces any breaking changes.

eladnava commented 6 years ago

@dmaltsiniotis Sounds great. @hypesystem Can we merge?

hypesystem commented 6 years ago

@eladnava could I get you to publish on npm? I'm not at a PC for a while

eladnava commented 6 years ago

@hypesystem Definitely, just published 0.14.10. 😄

hypesystem commented 6 years ago

Great :-) Thanks for the contribution, @dmaltsiniotis !