toymachiner62 / hapi-authorization

ACL plugin for hapijs
MIT License
80 stars 25 forks source link

Support for Hapi v17 #24

Closed toddhickerson closed 6 years ago

toddhickerson commented 7 years ago

This plugin needs changes to the register method to support Hapi v17.

https://github.com/hapijs/hapi/issues/3658

toddhickerson commented 6 years ago

Hey @toymachiner62, are you still interested in maintaining this library? There are breaking changes in Hapi v17 that change the way plugins are handled, rendering this library unusable in Hapi v17. I'm looking for alternatives or will need to fork and update myself.

Lastly, are you looking for pull requests to fix this...and if so, will it be considered?

I really find the module useful and would hate to see it go away.

Thank you.

toymachiner62 commented 6 years ago

Yes still interested in maintaining although I haven't used hapi.js in any projects recently.

A pull request would really help me out.

Tom Caflisch 507.202.0087 TomCaflisch@gmail.com http://allthingswebdesign.com

On Sat, Dec 9, 2017 at 2:12 PM, Todd Hickerson notifications@github.com wrote:

Hey @toymachiner62 https://github.com/toymachiner62, are you still interested in maintaining this library? There are breaking changes in Hapi v17 that change the way plugins are handled, rendering this library unusable in Hapi v17. I'm looking for alternatives or will need to fork and update myself.

Lastly, are you looking for pull requests to fix this...and if so, will it be considered?

I really find the module useful and would hate to see it go away.

Thank you.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/toymachiner62/hapi-authorization/issues/24#issuecomment-350502402, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdplq56FX9SpMmoa-G77dJ4T5dRBGYrks5s-um3gaJpZM4Qahid .

NicolasRitouet commented 6 years ago

I've started the work to migrate to hapi 17 on this branch: https://github.com/mycsHQ/hapi-authorization/compare/master...hapi-17 Tests are not passing yet and next() have to been removed from the codebase. I also removed q

maxaus commented 6 years ago

@NicolasRitouet Please, see my pull-request to your fork - I recently finished work on Hapi v17 related updates, but initially based on your branch. So it makes sence to merge my pull request to your repo (if it's ok), then make a pull request to this original repo.

NicolasRitouet commented 6 years ago

@maxaus thx a lot for your help 🎉 I've merged your PR into my branch. 👍🏻 I just got back from vacation, give me a few days to test it on my side and then create a PR to the original repo.

heartstchr commented 6 years ago

@NicolasRitouet i am still getting [1] "name" is required is this repo supports hapi 17 @maxaus Is this issue resolved? I am still facing the issue

toymachiner62 commented 6 years ago

Still waiting on a PR from @NicolasRitouet

NicolasRitouet commented 6 years ago

@toymachiner62 I've created the PR, but I had no time to test it properly. 😕 I've tested it and it works for my use-case. If someone can, you can just pull from the branch hapi-17 of my fork (https://github.com/mycsHQ/hapi-authorization/tree/hapi-17).

devinceble commented 6 years ago

any news with the merge? my work around is using https://github.com/mycsHQ/hapi-authorization/tree/hapi-17-2 works with hapi v17.5.4 thanks @NicolasRitouet

toymachiner62 commented 6 years ago

@devinceble Thanks for the nudge. I've got this into NPM as version 4.0.0 now.

Also since i've not been using hapi.js the past couple years i'm way out of the loop with it. Any interest in any of you becoming a co-maintainer of this lib?