-
Notifications
You must be signed in to change notification settings - Fork 192
Conversation
var Hapi = require('hapi'); | ||
|
||
var internals = { | ||
pkg: require('./package.json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to the overview submissions should be using the hapi style guide, which specifies 4 spaces for rather than tabs for indents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the head up, I'll have to change IDE settings for that, no problem
And I didn't know that guide existed O_o
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no probs 😄
return reply({ version: internals.pkg.version }); | ||
}, | ||
config: { | ||
description: 'Returns the version of the server' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice touch! If you have config
, it's cleaner to put handler
inside.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by config? And yes once the routes get more complex the handlers will be moved out of the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe he means that it makes sense to do
server.route({
method: 'GET',
path: '/version',
config: {
description: 'Returns the version of the server',
handler: function (request, reply) {
return reply({ version: internals.pkg.version });
}
}
});
I typically move my handlers to a file ./handlers.js
, so it looks like this in my index.js
server.route({
method: 'GET',
path: '/version',
config: handlers.version
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, yes I also move them to a separate place. Thanks for clearing that up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hueniverse is that a general rule? To put handler in config if it is present? It will probably be cleaner once the handler is in a separate place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I like to do. More readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
assertion // updated license // updated gitignore file (taken from hapi repo)
@@ -0,0 +1 @@ | |||
module.exports = require('./lib/index.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason to export anything here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just require('./lib/index.js');
then? Or even require('./lib')
although i'm not familiar with what it will do then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok found it in the docs will patch
[Assignment 1] Basic server
@hueniverse awesome thanks! If you want them squashed next time let me know :p |
Closes #1