search

hemerajs / hemera

🔬 Writing reliable & fault-tolerant microservices in Node.js https://hemerajs.github.io/hemera/
MIT License
806 stars 70 forks source link

Update Hapi Dependencies #243

Closed jackieluc closed 5 years ago

jackieluc commented 5 years ago

Hi @StarpTech,

The following hapi packages have been deprecated and have been migrated under the hapi organization. I thought it would be great to migrate them (so they continue to be supported) and update the dependencies.

Old Version New Version Notes
code v^4.1.0 @hapi/code v^6.0.0
hoek v4.2.x @hapi/hoek v8.3.x
heavy v4.0.x @hapi/heavy v6.2.x Updated respective config schema, API, and tests
joi v^12.0.0 @hapi/joi v^15.1.1 I decided not to upgrade to v16 because v16 has breaking changes that require a lot of effort and complexity to upgrade: https://github.com/hapijs/joi/issues/2037

I also bumped the versions of each of the following packages that were affected by a minor version (following semver). Please advise if you'd like things to be different.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.003%) to 96.634% when pulling f258d3fe514dcdb96a552d651ada85d124523193 on momiller121:master into bf05fd268ce1386eaabec05c874c36c368b3120b on hemerajs:master.

jackieluc commented 5 years ago

The Travis CI has failed due to dependency updates using new features of JavaScript that aren't supported in Node 6. Should we consider dropping support for Node 6 as it reached end of life on 2019-04-30? https://github.com/nodejs/Release#release-schedule

momiller121 commented 5 years ago

@jackieluc - dropping Node6 support was one of starptech's stated goals.

StarpTech commented 5 years ago

Hi @jackieluc thank you for the PR. This PR can't be merged as long as we support Node.js 6 which will introduce a breaking change.

momiller121 commented 5 years ago

@starptech - if we resubmit this PR with node 6 removed, is this a helpful contribution towards hemera@8? We're open to being coached / advised how we can best contribute. Thank you.

StarpTech commented 5 years ago

This PR hasn't a single purpose. There are too many breaking changes. Please create several PR's.

if we resubmit this PR with node 6 removed, is this a helpful contribution towards hemera@8? We're open to being coached / advised how we can best contribute. Thank you.

Yes, if it really contains only a dependency update. I will create a branch called next which prepares for the next major version.

jackieluc commented 5 years ago

Its my bad for making a PR without checking Node 6 support first - though it seems like the migrated Hapi dependencies do not support Node 6. I agree that this PR would be more productive if it were to be included in the next major version of Hemera that drops support for unsupported Node versions.

momiller121 commented 5 years ago

Hello @StarpTech - I propose that this is a singular purpose PR based on the following rationale: This PR resolves all hapi family component deprecation warnings. (Dropping of Node6 support is also implied to affect this.) Otherwise, the existing functional contract is unchanged.

So, breaking change for dropping Node6 - yes. Otherwise, no breaking change.

And, in a subsequent PR, we'll add Node@12 support (though I have not even looked at that yet).

StarpTech commented 5 years ago

@momiller121 that's not correct. Following changes are breaking changes beside to support new hapi modules:

@jackieluc please create different PR's for that and merge against next not master.

momiller121 commented 5 years ago

@StarpTech - I see that now. Thank you. We'll reformulate against the next branch.

StarpTech commented 5 years ago

I really appreciate that PR but I think it's not worth it. I will update all dependencies in a batch. In the next days, I will assign several issues to the milestone Hemera 8 Pheonix. Feel free to pick one :relaxed: