{"action":"created","issue":{"url":"https://api.github.com/repos/thaliproject/postcardapp/issues/30","labels_url":"https://api.github.com/repos/thaliproject/postcardapp/issues/30/labels{/name}","comments_url":"https://api.github.com/repos/thaliproject/postcardapp/issues/30/comments","events_url":"https://api.github.com/repos/thaliproject/postcardapp/issues/30/events","html_url":"https://github.com/thaliproject/postcardapp/pull/30","id":103493461,"number":30,"title":"Story 0 - Polymer WebApp","user":{"login":"deadlyfingers","id":1880480,"avatar_url":"https://avatars.githubusercontent.com/u/1880480?v=3","gravatar_id":"","url":"https://api.github.com/users/deadlyfingers","html_url":"https://github.com/deadlyfingers","followers_url":"https://api.github.com/users/deadlyfingers/followers","following_url":"https://api.github.com/users/deadlyfingers/following{/other_user}","gists_url":"https://api.github.com/users/deadlyfingers/gists{/gist_id}","starred_url":"https://api.github.com/users/deadlyfingers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/deadlyfingers/subscriptions","organizations_url":"https://api.github.com/users/deadlyfingers/orgs","repos_url":"https://api.github.com/users/deadlyfingers/repos","events_url":"https://api.github.com/users/deadlyfingers/events{/privacy}","received_events_url":"https://api.github.com/users/deadlyfingers/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/thaliproject/postcardapp/labels/in%20progress","name":"in progress","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2015-08-27T12:03:52Z","updated_at":"2015-08-27T23:17:53Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/thaliproject/postcardapp/pulls/30","html_url":"https://github.com/thaliproject/postcardapp/pull/30","diff_url":"https://github.com/thaliproject/postcardapp/pull/30.diff","patch_url":"https://github.com/thaliproject/postcardapp/pull/30.patch"},"body":"One additional setup command:\r\n- run `bower install` in www/jxcore directory\r\n\r\nNotes: \r\nCordova config.xml should install 'cordova-plugin-statusbar' plugin.\r\nUses iOS Thali plugin branch.\n<a href='#crh-start'></a><a href='#crh-data-%7B%7D'></a>\n<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>\n<a href='https://www.codereviewhub.com/thaliproject/postcardapp/pull/30?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/thaliproject/postcardapp/pull/30'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>\n<a href='#crh-end'></a>\n\n<!-- Reviewable:start -->\n[<img src=\"https://reviewable.io/review_button.png\" height=40 alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/thaliproject/postcardapp/30)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/thaliproject/postcardapp/issues/comments/135580789","html_url":"https://github.com/thaliproject/postcardapp/pull/30#issuecomment-135580789","issue_url":"https://api.github.com/repos/thaliproject/postcardapp/issues/30","id":135580789,"user":{"login":"yaronyg","id":1480964,"avatar_url":"https://avatars.githubusercontent.com/u/1480964?v=3","gravatar_id":"","url":"https://api.github.com/users/yaronyg","html_url":"https://github.com/yaronyg","followers_url":"https://api.github.com/users/yaronyg/followers","following_url":"https://api.github.com/users/yaronyg/following{/other_user}","gists_url":"https://api.github.com/users/yaronyg/gists{/gist_id}","starred_url":"https://api.github.com/users/yaronyg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yaronyg/subscriptions","organizations_url":"https://api.github.com/users/yaronyg/orgs","repos_url":"https://api.github.com/users/yaronyg/repos","events_url":"https://api.github.com/users/yaronyg/events{/privacy}","received_events_url":"https://api.github.com/users/yaronyg/received_events","type":"User","site_admin":false},"created_at":"2015-08-27T23:17:53Z","updated_at":"2015-08-27T23:17:53Z","body":"\n\n\n\nReviewed 231 of 244 files at r1.\nReview status: 231 of 244 files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.\n\n---\n\n<sup>**[www/jxcore/cardroutes.js, line 16 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/postcardapp/30#-JxlIaZYE9Jq7BhpmBGs)** ([raw file](https://github.com/thaliproject/postcardapp/blob/cac20290092ee1960d27611e7366710d3e6af715/www/jxcore/cardroutes.js#L16)):</sup>\nPlease remove the comments such as this one and the ones below\n\n---\n\n<sup>**[www/jxcore/cardroutes.js, line 23 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/postcardapp/30#-JxlIzs95Zy8jUhzDLOi)** ([raw file](https://github.com/thaliproject/postcardapp/blob/cac20290092ee1960d27611e7366710d3e6af715/www/jxcore/cardroutes.js#L23)):</sup>\nIt's generally considered to be bad style to nest ifs like this. Typically you would have a if test test for userName.length <= 0 and then immediately do the res.send with user name is required followed by a return exiting the function. Then you would have the db.get and inside of it the first if (on err & err.status) would also end with a return. This would let you out dent a bunch of stuff. Same thing all the way down. E.g. the if (err) body would end in a return as well.\n\n---\n\n<sup>**[www/jxcore/public/elements/behaviors/multi-platform.html, line 34 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/postcardapp/30#-JxlMyitD_A0IgYEx7d4)** ([raw file](https://github.com/thaliproject/postcardapp/blob/cac20290092ee1960d27611e7366710d3e6af715/www/jxcore/public/elements/behaviors/multi-platform.html#L34)):</sup>\nDo we really want to log this every time this function is called? In general we should review the console statements and make sure they are actually useful. We don't want to fill up the log with a bunch of not terribly useful stuff and then hide the useful stuff. You should take a look at Winston's logging levels. It would let you log things like this but have them set to 'informational' and be ignored by default.\n\n---\n\n<sup>**[www/jxcore/public/elements/behaviors/path-components.html, line 33 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/postcardapp/30#-JxlNwqj9_5vl726KGuC)** ([raw file](https://github.com/thaliproject/postcardapp/blob/cac20290092ee1960d27611e7366710d3e6af715/www/jxcore/public/elements/behaviors/path-components.html#L33)):</sup>\nNot to be too simplistic but um... I would expect a function called _isInt to use parseInt(value, 10) not parseFloat. Is there browser magic here I'm not aware of?\n\n---\n\n<sup>**[www/jxcore/public/elements/behaviors/tree-traversing.html, line 20 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/postcardapp/30#-JxlOjuG0hdndf-8uolu)** ([raw file](https://github.com/thaliproject/postcardapp/blob/cac20290092ee1960d27611e7366710d3e6af715/www/jxcore/public/elements/behaviors/tree-traversing.html#L20)):</sup>\nInstead of an if else if tree wouldn't switch do a better job?\n\n---\n\n\nComments from the [review on Reviewable.io](https://reviewable.io:443/reviews/thaliproject/postcardapp/30)\n<!-- Sent from Reviewable.io -->\n"}}