Update to hapi v17. - Githubissues

WesTyler commented 6 years ago

Updates to hapi v17 and lab v15

Breaking Changes:

Remove "connections" and "onPreConnection" from manifest.
Return async function instead of using a callback or native Promise.
Change "preRegister" contract from callback to async function.
Change manifest shape from manifest.registrations array to manifest.register.plugins array.
Change expected plugin object shape to match hapi v17 expectations.

Non-Breaking Changes:

Hand plugin and registration options straight to hapi for validation at registration time.
Single server.register call instead of one per plugin.

WesTyler commented 6 years ago

Okay, I took a first pass at the changes we discussed above. I also tried to update the API.md to match. I don't love the wording in some parts, but I could use some input on how to make it more clear.

Is this implementation what you had in mind?

csrl commented 6 years ago

I pushed a work in progress review commit to 'review/pr104' branch to give you an idea of how I'm looking at it. If that makes sense to you, and you want to finish it off (updating the rest of the tests etc), please do.

WesTyler commented 6 years ago

Ohhh excellent yes. I'm actually very happy to be removing Items and going to async/await. :+1: :+1: :+1:

I'll wrap those changes in here this afternoon. Thanks!

WesTyler commented 6 years ago

Alright, I did the async/await refactor and also went ahead and updated dev dependencies to lab@15 and code@5 to make use of the async/await features there.

WesTyler commented 6 years ago

Done and done! Thanks for the help getting this where it needed to be @csrl

csrl commented 6 years ago

I think I will hold off on releasing this until hapi 17 final lands. We could do a release candidate, but I'm not too motivated to do so. So if you want to update the package.json to reference hapi 17 instead of rc9, then it'll be a simple merge whenever hapi 17 is released. thoughts?

WesTyler commented 6 years ago

Yeah, that sounds good to me :+1:

I'll keep an eye on the state of the branch if Eran does anymore rcs just to make sure.

WesTyler commented 6 years ago

Also, the Travis build is going to fail since there is no install target yet for "hapi": "17.x.x". Just throwing that out there before a big red X appears next to my commit.

WesTyler commented 6 years ago

Looks like we are all good with the release of 17.0.0 yesterday.

csrl commented 6 years ago

Thanks @WesTyler. Nice work on this release. I'm actually not sure when I'll be updating to hapi 17 for the main project I work on with hapi. Do you have any interesting in taking over as maintainer for glue?

WesTyler commented 6 years ago

Nice work on this release

Thank you, and thank you for the help with it!

Do you have any interesting in taking over as maintainer for glue?

If you are wanting to pass it off, then absolutely! We will be updating to hapi 17 and glue 5 at work soon, so I should have plenty of time to be hands on with it.

csrl commented 6 years ago

ok, I'll let you know - we'll make a decision next week on when/if we will port to hapi 17.

WesTyler commented 6 years ago

Sounds good :+1: On Mon, Nov 6, 2017 at 6:13 PM csrl notifications@github.com wrote:

ok, I'll let you know - we'll make a decision next week on when/if we will port to hapi 17.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hapijs/glue/pull/104#issuecomment-342331640, or mute the thread https://github.com/notifications/unsubscribe-auth/ALl4O_6Gap_jf_3zAm5fUWJoXxrrFflzks5sz6DDgaJpZM4QBn08 .

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

hapijs / glue

Update to hapi v17. #104