Bug 63965 - garden-o-matic should understand which tests have unexpected failures
Summary: garden-o-matic should understand which tests have unexpected failures
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-05 18:42 PDT by Adam Barth
Modified: 2011-07-05 19:32 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.36 KB, patch)
2011-07-05 18:43 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.