New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Now works with hapi 8 and the latest moonboots_hapi #87

Merged
merged 4 commits into from Jan 7, 2015

Conversation

Projects
None yet
4 participants
@geek
Contributor

geek commented Jan 6, 2015

Closes #85

});
// require moonboots_hapi plugin
server.pack.register({plugin: require('moonboots_hapi'), options: moonbootsConfig}, function (err) {
server.register({ register: require('moonboots_hapi').register, options: moonbootsConfig }, function (err) {

This comment has been minimized.

@wraithgar

wraithgar Jan 6, 2015

Member

the .register after require isn't needed.

@wraithgar

wraithgar Jan 6, 2015

Member

the .register after require isn't needed.

This comment has been minimized.

@geek

geek Jan 6, 2015

Contributor

It is in hapi 8: http://hapijs.com/api#serverregisterplugins-options-callback

If you change moonboots_hapi to module.exports the register function it won't be though: https://github.com/wraithgar/moonboots_hapi/blob/master/index.js#L43

@geek

geek Jan 6, 2015

Contributor

It is in hapi 8: http://hapijs.com/api#serverregisterplugins-options-callback

If you change moonboots_hapi to module.exports the register function it won't be though: https://github.com/wraithgar/moonboots_hapi/blob/master/index.js#L43

This comment has been minimized.

@wraithgar

wraithgar Jan 6, 2015

Member

Yep you're right, it works the other way too but the docs show your way.

@wraithgar

wraithgar Jan 6, 2015

Member

Yep you're right, it works the other way too but the docs show your way.

@wraithgar

This comment has been minimized.

Show comment
Hide comment
@wraithgar

wraithgar Jan 6, 2015

Member

+1 visual w/ only one minor nitpick

Member

wraithgar commented Jan 6, 2015

+1 visual w/ only one minor nitpick

@geek

This comment has been minimized.

Show comment
Hide comment
@geek

geek Jan 6, 2015

Contributor

Interested in a .travis.yml?

Contributor

geek commented Jan 6, 2015

Interested in a .travis.yml?

@wraithgar

This comment has been minimized.

Show comment
Hide comment
@wraithgar

wraithgar Jan 6, 2015

Member

My gut feeling is that is more of an examples addition, this demo app is supposed to just do a quick wireframe so people can see the basics of ampersand code itself.

Member

wraithgar commented Jan 6, 2015

My gut feeling is that is more of an examples addition, this demo app is supposed to just do a quick wireframe so people can see the basics of ampersand code itself.

@geek

This comment has been minimized.

Show comment
Hide comment
@geek

geek Jan 6, 2015

Contributor

Sounds good. Might as well remove the travis integration with this repo then.

Contributor

geek commented Jan 6, 2015

Sounds good. Might as well remove the travis integration with this repo then.

@fyockm

This comment has been minimized.

Show comment
Hide comment
@fyockm

fyockm Jan 6, 2015

Member

@bear should travis be removed from this repo?
@wraithgar did you mean that this repo should not be updated to hapi 8?

Member

fyockm commented Jan 6, 2015

@bear should travis be removed from this repo?
@wraithgar did you mean that this repo should not be updated to hapi 8?

@bear

This comment has been minimized.

Show comment
Hide comment
@bear

bear Jan 6, 2015

Contributor

@fyockm yes, please do remove it - it got added by one of my automation scripts that I've yet tweaked to have per repo exclusions for integrations

/edit I just removed it - sorry for the noise

Contributor

bear commented Jan 6, 2015

@fyockm yes, please do remove it - it got added by one of my automation scripts that I've yet tweaked to have per repo exclusions for integrations

/edit I just removed it - sorry for the noise

@wraithgar

This comment has been minimized.

Show comment
Hide comment
@wraithgar

wraithgar Jan 6, 2015

Member

@fyockm =this repo should definitely be updated to hapi8

Member

wraithgar commented Jan 6, 2015

@fyockm =this repo should definitely be updated to hapi8

@fyockm

This comment has been minimized.

Show comment
Hide comment
@fyockm

fyockm Jan 6, 2015

Member

@wraithgar ok, I must have misunderstood your comment. So what did you mean should be an examples addition? The travis.yml?

Member

fyockm commented Jan 6, 2015

@wraithgar ok, I must have misunderstood your comment. So what did you mean should be an examples addition? The travis.yml?

@wraithgar

This comment has been minimized.

Show comment
Hide comment
@wraithgar

wraithgar Jan 6, 2015

Member

Yeah, unless I'm misunderstanding what that file will do (which is very likely)

Member

wraithgar commented Jan 6, 2015

Yeah, unless I'm misunderstanding what that file will do (which is very likely)

@fyockm

This comment has been minimized.

Show comment
Hide comment
@fyockm

fyockm Jan 7, 2015

Member

👍 this LGTM

@wraithgar we have disabled travis in the AmpersandJS/examples repo, as tests are not required with submitted examples. it might be a good idea to ask for tests with the examples, but I think we didn't want to stifle contribution from the community (at least in the beginning).

Member

fyockm commented Jan 7, 2015

👍 this LGTM

@wraithgar we have disabled travis in the AmpersandJS/examples repo, as tests are not required with submitted examples. it might be a good idea to ask for tests with the examples, but I think we didn't want to stifle contribution from the community (at least in the beginning).

bear added a commit that referenced this pull request Jan 7, 2015

Merge pull request #87 from geek/master
Now works with hapi 8 and the latest moonboots_hapi

@bear bear merged commit f2fa00c into AmpersandJS:master Jan 7, 2015

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
@wraithgar

This comment has been minimized.

Show comment
Hide comment
@wraithgar

wraithgar Jan 7, 2015

Member

Thank you @geek!

Member

wraithgar commented Jan 7, 2015

Thank you @geek!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment