{"action":"created","issue":{"url":"https://api.github.com/repos/ripple/ripple-lib/issues/242","labels_url":"https://api.github.com/repos/ripple/ripple-lib/issues/242/labels{/name}","comments_url":"https://api.github.com/repos/ripple/ripple-lib/issues/242/comments","events_url":"https://api.github.com/repos/ripple/ripple-lib/issues/242/events","html_url":"https://github.com/ripple/ripple-lib/pull/242","id":52832349,"number":242,"title":"[TASK] Update jshintrc","user":{"login":"clark800","id":1238722,"avatar_url":"https://avatars.githubusercontent.com/u/1238722?v=3","gravatar_id":"","url":"https://api.github.com/users/clark800","html_url":"https://github.com/clark800","followers_url":"https://api.github.com/users/clark800/followers","following_url":"https://api.github.com/users/clark800/following{/other_user}","gists_url":"https://api.github.com/users/clark800/gists{/gist_id}","starred_url":"https://api.github.com/users/clark800/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/clark800/subscriptions","organizations_url":"https://api.github.com/users/clark800/orgs","repos_url":"https://api.github.com/users/clark800/repos","events_url":"https://api.github.com/users/clark800/events{/privacy}","received_events_url":"https://api.github.com/users/clark800/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2014-12-24T18:48:00Z","updated_at":"2015-01-06T17:26:55Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ripple/ripple-lib/pulls/242","html_url":"https://github.com/ripple/ripple-lib/pull/242","diff_url":"https://github.com/ripple/ripple-lib/pull/242.diff","patch_url":"https://github.com/ripple/ripple-lib/pull/242.patch"},"body":"@wltsmrz Thanks for polishing up the jshintrc file. There are just two edits that I'd like to add: removing the `boss` and `W004` relaxing options. I also moved `laxbreak` to the end, that is one relaxing option that may have some readability benefits. And I removed `camelcase` since it has no effect.\r\n\r\nI can't think of any cases where `boss` (assignments where comparisons are expected) or `W004` (redefining parameters) should be allowed, but if there is a rare case where someone really wants to use it, they can disable the jshint warning with the `// jshint ignore:line` comment directive. But I'd imagine that 99% of the time when these warnings fire it will be catching bugs for us."},"comment":{"url":"https://api.github.com/repos/ripple/ripple-lib/issues/comments/68898552","html_url":"https://github.com/ripple/ripple-lib/pull/242#issuecomment-68898552","issue_url":"https://api.github.com/repos/ripple/ripple-lib/issues/242","id":68898552,"user":{"login":"geertweening","id":966236,"avatar_url":"https://avatars.githubusercontent.com/u/966236?v=3","gravatar_id":"","url":"https://api.github.com/users/geertweening","html_url":"https://github.com/geertweening","followers_url":"https://api.github.com/users/geertweening/followers","following_url":"https://api.github.com/users/geertweening/following{/other_user}","gists_url":"https://api.github.com/users/geertweening/gists{/gist_id}","starred_url":"https://api.github.com/users/geertweening/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/geertweening/subscriptions","organizations_url":"https://api.github.com/users/geertweening/orgs","repos_url":"https://api.github.com/users/geertweening/repos","events_url":"https://api.github.com/users/geertweening/events{/privacy}","received_events_url":"https://api.github.com/users/geertweening/received_events","type":"User","site_admin":false},"created_at":"2015-01-06T17:26:55Z","updated_at":"2015-01-06T17:26:55Z","body":"> It's easy to read and hard to misunderstand, to me. It's unlikely that anyone is going to use this without knowing what they're doing. Making jshint more strict in this regard won't catch any bugs\r\n\r\n```\r\n if ((result = cache[hash])) {\r\n    break;\r\n }\r\n```\r\nIf I look at this case, I'm not sure if the assignment of `result` returns a boolean or if the test is just for a truthy value. In this case the assignment returns a transaction, so it's testing for presence. That's not clear from the snippet, you would have to look elsewhere in the code to see what's going on exactly. IMO this would be more readable and clear:\r\n\r\n```\r\nresult = cache[hash];\r\nif (result !== void(0)) {\r\n...\r\n```\r\nOr, if you only care about truthy values\r\n```\r\nresult = cache[hash];\r\nif (result) {\r\n...\r\n```\r\n\r\nI don't really like relying on implicit returns, specially when assigning and testing the result in a statement like this. I would rather be a little more verbose for the sake of clarity. \r\n\r\nI would prefer to take out the `boss` options so we don't write this code, but I care most about agreeing what is a better way to write a case like this. I personally feel the implicit return and assignment in an if case is not the best way.\r\n"}}