{"action":"created","issue":{"url":"https://api.github.com/repos/crafatar/crafatar/issues/179","repository_url":"https://api.github.com/repos/crafatar/crafatar","labels_url":"https://api.github.com/repos/crafatar/crafatar/issues/179/labels{/name}","comments_url":"https://api.github.com/repos/crafatar/crafatar/issues/179/comments","events_url":"https://api.github.com/repos/crafatar/crafatar/issues/179/events","html_url":"https://github.com/crafatar/crafatar/pull/179","id":157429640,"number":179,"title":"First pass at ES6 refactor","user":{"login":"RobertHerhold","id":8069753,"avatar_url":"https://avatars.githubusercontent.com/u/8069753?v=3","gravatar_id":"","url":"https://api.github.com/users/RobertHerhold","html_url":"https://github.com/RobertHerhold","followers_url":"https://api.github.com/users/RobertHerhold/followers","following_url":"https://api.github.com/users/RobertHerhold/following{/other_user}","gists_url":"https://api.github.com/users/RobertHerhold/gists{/gist_id}","starred_url":"https://api.github.com/users/RobertHerhold/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RobertHerhold/subscriptions","organizations_url":"https://api.github.com/users/RobertHerhold/orgs","repos_url":"https://api.github.com/users/RobertHerhold/repos","events_url":"https://api.github.com/users/RobertHerhold/events{/privacy}","received_events_url":"https://api.github.com/users/RobertHerhold/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-05-30T04:32:51Z","updated_at":"2016-05-30T06:29:06Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/crafatar/crafatar/pulls/179","html_url":"https://github.com/crafatar/crafatar/pull/179","diff_url":"https://github.com/crafatar/crafatar/pull/179.diff","patch_url":"https://github.com/crafatar/crafatar/pull/179.patch"},"body":"I initially started doing this all in one go, but quickly realized that the project was far too large to do one complete refactor. Thus, I have decided to do this with an iterative approach, given that I receive good feedback.\r\n\r\nThe goal of this PR (and the ones to come) is to refactor the project with ES6 capabilities, namely promises and generators, to result in cleaner code that is easier to read. Although this PR does not deal much with changing the callbacks to promises, it does change the software the server runs on to [koa](https://github.com/koajs/koa), a modern approach to a Node.js server. With koa, there are many improvements that can be made to this project (many of which are not included in this PR):\r\n * [Context](https://github.com/koajs/koa/blob/master/docs/api/context.md) for each request - this makes it so that we can throw an error from anywhere and the logger middleware would be aware of the request (and its ID)\r\n\r\n* No callback hell - by replacing callback functions with promise-returning functions, we can avoid messy code such as [here](https://github.com/crafatar/crafatar/blob/6c132f5c51a3b0a05abd3b07dc6776d80c5240a5/lib/helpers.js#L65-L74)\r\n\r\n* Less mucking around with request and response objects - by using Koa, we can avoid a lot of the manual labor such as parsing query/path strings, and in general, just shorten the amount of code that needs to be written. One example of this is that in server.js, I have managed to shorten the number of lines by almost 50% while also improving readability\r\n\r\nAfter this PR, the project is a mix of the old and new style, but I hope that by the end of this refactor, it will be almost entirely promisified, and that a lot of unnecessary (in the future) code such as response.js can be eliminated or condensed.\r\n\r\nPlease, let me know what you think."},"comment":{"url":"https://api.github.com/repos/crafatar/crafatar/issues/comments/222420532","html_url":"https://github.com/crafatar/crafatar/pull/179#issuecomment-222420532","issue_url":"https://api.github.com/repos/crafatar/crafatar/issues/179","id":222420532,"user":{"login":"Jake0oo0","id":1993773,"avatar_url":"https://avatars.githubusercontent.com/u/1993773?v=3","gravatar_id":"","url":"https://api.github.com/users/Jake0oo0","html_url":"https://github.com/Jake0oo0","followers_url":"https://api.github.com/users/Jake0oo0/followers","following_url":"https://api.github.com/users/Jake0oo0/following{/other_user}","gists_url":"https://api.github.com/users/Jake0oo0/gists{/gist_id}","starred_url":"https://api.github.com/users/Jake0oo0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Jake0oo0/subscriptions","organizations_url":"https://api.github.com/users/Jake0oo0/orgs","repos_url":"https://api.github.com/users/Jake0oo0/repos","events_url":"https://api.github.com/users/Jake0oo0/events{/privacy}","received_events_url":"https://api.github.com/users/Jake0oo0/received_events","type":"User","site_admin":false},"created_at":"2016-05-30T06:29:06Z","updated_at":"2016-05-30T06:29:06Z","body":"So in #56, we originally removed Express due to the unnecessary overhead, and the fact that we only needed a very simple http server anyway. I haven't been able to properly benchmark your version because I'm getting this error:\r\n\r\n```bash\r\n  xxx GET /avatars/Daan_nos?overlay 500 262ms -\r\n\r\n  TypeError: Cannot read property 'body' of undefined\r\n      at Object.module.exports.next (/home/jake88/node/crafatar/lib/response.js:31:32)\r\n      at next (native)\r\n      at onFulfilled (/home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:65:19)\r\n      at /home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:54:5\r\n      at Object.co (/home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:50:10)\r\n      at Object.toPromise (/home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:118:63)\r\n      at next (/home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:99:29)\r\n      at onFulfilled (/home/jake88/node/crafatar/node_modules/koa/node_modules/co/index.js:69:7)\r\n      at tryCallOne (/home/jake88/node/crafatar/node_modules/promisify-node/node_modules/nodegit-promise/lib/core.js:37:12)\r\n      at /home/jake88/node/crafatar/node_modules/promisify-node/node_modules/nodegit-promise/lib/core.js:103:15\r\n```\r\n\r\nfor a large portion of the requests. I wouldn't necessarily be against using Koa this time around, but if it does increase memory usage/increases request times, I don't think it's really necessary because of the same reasons that #56 was implemented. Maybe @jomo has a more definitive answer in regards to using Koa."}}
{"actor":{"display_login":"Jake0oo0"}}