garden-o-matic should understand which tests have unexpected failures
Created attachment 99771 [details] Patch
Comment on attachment 99771 [details] Patch I'm not a very good JS reviewer. You owe me some unit testing....
Why is our JS style 2-space indent? Does check-webkit-style enforce that?
> Why is our JS style 2-space indent? Does check-webkit-style enforce that? This style matches the code-review.js code. I don't think it's enforced by check-webkit-style.
Comment on attachment 99771 [details] Patch Seems fine to me. I'm not familiar with jquery grep, but I'll assume it does what you want. I'd just use "filter" on the array, but I know you luvs ur jquery. :) I should have commented on the previous patch as well, I don't like using a different style for our JS code than our C++/Python code. The benefits are not worth the context switching cost. I'd prefer if we did 4 space indent and open curly on a new line.
(In reply to comment #4) > > Why is our JS style 2-space indent? Does check-webkit-style enforce that? > > This style matches the code-review.js code. I don't think it's enforced by check-webkit-style. We should change that as well IMO.
> I should have commented on the previous patch as well, I don't like using a different style for our JS code than our C++/Python code. The benefits are not worth the context switching cost. I'd prefer if we did 4 space indent and open curly on a new line. Python style is a bad match for JavaScript because the JavaScript standard library doesn't match and that's odd. I'll change the style to match our C++ style (moving the curlies and camelCasing the variables, etc).
Comment on attachment 99771 [details] Patch Clearing flags on attachment: 99771 Committed r90428: <http://trac.webkit.org/changeset/90428>
All reviewed patches have been landed. Closing bug.