search

funktionswerk / hapi-i18n

Translation module for hapi based on mashpie's i18n module
MIT License
39 stars 22 forks source link

Hapi17 compatible #20

Closed rapoell closed 6 years ago

rapoell commented 6 years ago

Changes required to be used by Hapi 17

kaywolter commented 6 years ago

Hi Ronald,

Thanks a lot for contributing.. Before I merge the PR: Could you please update the npm dependencies and the unit tests?

Thanks, Kay

rapoell commented 6 years ago

Hi @kaywolter Working on it.

What shall I do with the version? Hapi 17 has breaking changes. There is no backward compatibility with earlier versions of Hapi. The ReadMe should also be updated for this.

kaywolter commented 6 years ago

I suggest to set the version to 2.0.0. Would be great if you update the README as well.

Thanks!

rapoell commented 6 years ago

@kaywolter I updated the code. As you suggested made it version 2.0.0. All tests pass. I'll see if I can add the Jade template test again (now it is handlebars). Can you please make a release of the current repo so it can be referenced and used by those who use Hapi < 17. I'm working on the ReadMe. Regards

rapoell commented 6 years ago

Updated the ReadMe too. I didn't check if the current versions of Jade and Nunjucks are correctly represented in the ReadMe. Please do so if you can. The version to be used by Hapi < 17 is not given yet (tbd - second line in the RM).

Observations are welcome. Regards

rapoell commented 6 years ago

Migrated to the new plugin API of Hapi 17.

kaywolter commented 6 years ago

I have changed the test cases to use async as this is the recommended approach for mocha and promises. I have also corrected the Handlebars sample (the helper was registered in the request handler) and refined the README. I think the PR is ready for merge now. Would be great if you have a final look at the changes: https://github.com/funktionswerk/hapi-i18n/pull/21

Thanks again for your contribution!

rapoell commented 6 years ago

Looks fine to me. Just a little explanation why I integrated a complete sample of the setup of the server in the RM. The documentation of Hapi 17 is lagging behind. The more complete examples are available the easier it will be for newcomers with this version to get started. But thats all. My style in separating the options from where they are used, is just my coding style. But perhaps it would be better if the way the options are used in the test and the way they are illustrated in the RM are identical. It was a pleasure to help with this.

kaywolter commented 6 years ago

Version 2.0.0 is published on npmjs. Thanks!

rapoell commented 6 years ago

Great.