Bug 63965

Summary: garden-o-matic should understand which tests have unexpected failures
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, dglazkov, eric, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Adam Barth 2011-07-05 18:42:09 PDT 
garden-o-matic should understand which tests have unexpected failures
Comment 1 Adam Barth 2011-07-05 18:43:35 PDT 
Created attachment 99771 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-07-05 18:44:48 PDT 
Comment on attachment 99771 [details]
Patch

I'm not a very good JS reviewer.  You owe me some unit testing....
Comment 3 Eric Seidel (no email) 2011-07-05 18:45:08 PDT 
Why is our JS style 2-space indent?  Does check-webkit-style enforce that?
Comment 4 Adam Barth 2011-07-05 18:45:40 PDT 
> 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 5 Ojan Vafai 2011-07-05 18:49:26 PDT 
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.
Comment 6 Ojan Vafai 2011-07-05 18:49:55 PDT 
(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.
Comment 7 Adam Barth 2011-07-05 18:52:48 PDT 
> 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 8 Adam Barth 2011-07-05 19:32:17 PDT 
Comment on attachment 99771 [details]
Patch

Clearing flags on attachment: 99771

Committed r90428: <http://trac.webkit.org/changeset/90428>
Comment 9 Adam Barth 2011-07-05 19:32:21 PDT 
All reviewed patches have been landed.  Closing bug.