{"action":"created","issue":{"url":"https://api.github.com/repos/phildawes/racer/issues/117","labels_url":"https://api.github.com/repos/phildawes/racer/issues/117/labels{/name}","comments_url":"https://api.github.com/repos/phildawes/racer/issues/117/comments","events_url":"https://api.github.com/repos/phildawes/racer/issues/117/events","html_url":"https://github.com/phildawes/racer/pull/117","id":54868695,"number":117,"title":"Review codecleaner: iterators/chars instead of while/bytes","user":{"login":"tafia","id":7351502,"avatar_url":"https://avatars.githubusercontent.com/u/7351502?v=3","gravatar_id":"","url":"https://api.github.com/users/tafia","html_url":"https://github.com/tafia","followers_url":"https://api.github.com/users/tafia/followers","following_url":"https://api.github.com/users/tafia/following{/other_user}","gists_url":"https://api.github.com/users/tafia/gists{/gist_id}","starred_url":"https://api.github.com/users/tafia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tafia/subscriptions","organizations_url":"https://api.github.com/users/tafia/orgs","repos_url":"https://api.github.com/users/tafia/repos","events_url":"https://api.github.com/users/tafia/events{/privacy}","received_events_url":"https://api.github.com/users/tafia/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-01-20T10:51:27Z","updated_at":"2015-01-20T16:02:04Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/phildawes/racer/pulls/117","html_url":"https://github.com/phildawes/racer/pull/117","diff_url":"https://github.com/phildawes/racer/pull/117.diff","patch_url":"https://github.com/phildawes/racer/pull/117.patch"},"body":"Hi,\r\n\r\nI did a review of codecleaner.\r\n\r\nThe main changes are :\r\n- change byte to chars in order to improve readability (i am not sure if performance is better or not, but i think this might be marginal)\r\n- change while loop to iterators. It feels more 'rust' this way. I think perf will be better (not that it mattered that much ... but why not !)\r\n- change ifs to match: again feels more 'rust' and improves readability (to me at least)\r\n\r\nI just tried to replicate the logic without having the very global picture. I trusted the tests to check if all is OK. \r\n\r\nLet me know if it helps or not and if i can continue on the other files."},"comment":{"url":"https://api.github.com/repos/phildawes/racer/issues/comments/70677755","html_url":"https://github.com/phildawes/racer/pull/117#issuecomment-70677755","issue_url":"https://api.github.com/repos/phildawes/racer/issues/117","id":70677755,"user":{"login":"phildawes","id":87502,"avatar_url":"https://avatars.githubusercontent.com/u/87502?v=3","gravatar_id":"","url":"https://api.github.com/users/phildawes","html_url":"https://github.com/phildawes","followers_url":"https://api.github.com/users/phildawes/followers","following_url":"https://api.github.com/users/phildawes/following{/other_user}","gists_url":"https://api.github.com/users/phildawes/gists{/gist_id}","starred_url":"https://api.github.com/users/phildawes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/phildawes/subscriptions","organizations_url":"https://api.github.com/users/phildawes/orgs","repos_url":"https://api.github.com/users/phildawes/repos","events_url":"https://api.github.com/users/phildawes/events{/privacy}","received_events_url":"https://api.github.com/users/phildawes/received_events","type":"User","site_admin":false},"created_at":"2015-01-20T16:02:04Z","updated_at":"2015-01-20T16:02:04Z","body":"Hi @tafia,\r\n\r\nI agree this looks nicer, thanks. \r\nHowever the reason codecleaner uses bytes is to avoid utf8 decoding (which the Chars iterator does), so I'd want to make sure the performance is acceptable before merging this patch.\r\nAlso I'm not sure it is correct - does chars().enumerate() return an iterator that enumerates the byte positions or just the index of the character?\r\n\r\nCheers,\r\n\r\nPhil\r\n"}}