Skip to content
This repository has been archived by the owner on Apr 8, 2019. It is now read-only.

[Assignment #1]: Basic HTTP Server #3

Closed
wants to merge 6 commits into from
Closed

[Assignment #1]: Basic HTTP Server #3

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d5a5acf
Assignment one: initial server
Mar 11, 2015
bcb0ee1
lib folder
Mar 11, 2015
9dfc023
Now .gitignore is only at the top level, changed the server variable …
muhammad-saleh Mar 11, 2015
a31611d
Adding one extra line to improve readability
muhammad-saleh Mar 12, 2015
574d6e3
Added .gitignore from hapijs/hapi, Changed package.json to reflect th…
muhammad-saleh Mar 16, 2015
0dbf4a4
Used Hoek and added new lines at the end
muhammad-saleh Mar 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
.DS_STORE
*.ipr
*.iml
*.iws
web/
lib/*.zip
version.properties
.sass-cache
swagger-ui.sublime-workspace
.idea
.project
node_modules/*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We like to copy the same ignore file for all hapi project and keep the consistent. Worth looking at hapijs/hapi to see what's in there.

19 changes: 19 additions & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var Hapi = require('hapi');
var packageJSON = require('./package.json');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better put as a key inside an internals object:

var internals = {
    version: require('./package.json').version
};

See style guide for more information about the use of internals.


var server = new Hapi.Server();
server.connection({ port: 8000 });

server.route({
method: 'GET',
path: '/version',
config: {
handler: function (request, reply) {
return reply({ version: packageJSON.version });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New line after function declaration.

}
}
});

server.start(function () {
console.log('Server running at:', server.info.uri);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check for start() errors. Also missing new line.

});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super bike sheddy, but I'm a stickler for white space. You might want to add an extra line here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I will do it.. but may I suggest we might use EditorConfig to unify the coding style ?
The main repo should have an editorconfig file and this will be reflected to Sublime automatically when you clone or fork the repo
This will help a lot to avoid such mistakes :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/editorconfig/editorconfig/wiki/Newline-at-End-of-File-Support

Fairly supported and a good idea

Here is an example one we can iterate from if you like --> https://github.com/Famous/famous/blob/develop/.editorconfig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard linting rules for Hapi can be found in Lab https://github.com/hapijs/lab/tree/master/lib/linters
Once tests are added these rules will probably be the standard

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint / jscs doesn't create the functionality that an .editorconfig does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true but it does add some 'restrictions'
using both is ideal

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, Do I need to do anything more here or it's now fine ?
Also I want to know what to do exactly when the assignment is done, Should you close the pull request or me ?
And when exactly should I consider the assignment as done ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assignment is due 17/3 and your PR will be closed either by being merged or probably be closed by @hueniverse

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool thanks a lot

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new line at the end of files

Empty file added lib/index.js
View file
Open in desktop
Empty file.
22 changes: 22 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "hueniversity",
"version": "0.0.1",
"description": "Assignment One",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "https://github.com/muhammad-saleh/hueniversity.git"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that once this is merged, it will be in this repo, not your fork. The package.json should reflect that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done... Please check the last commit when you have time
I will be more careful with the style guide in the future

},
"author": "",
"license": "ISC",
"bugs": {
"url": "https://github.com/muhammad-saleh/hueniversity/issues"
},
"homepage": "https://github.com/muhammad-saleh/hueniversity",
"dependencies": {
"hapi": "^8.3.1"
}
}