| Summary: | webkit-perf.appspot.com should have an ability to merge tests | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
| Component: | WebKit Website | Assignee: | Ryosuke Niwa <rniwa> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, eric, ojan, tony, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 77037 | ||||||
| Attachments: |
|
||||||
|
Description
Ryosuke Niwa
2012-01-27 15:22:08 PST
Created attachment 124388 [details]
Adds admin/merge-tests
Attachment 124388 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Websites/webkit-perf.appspot..." exit_code: 1
Websites/webkit-perf.appspot.com/merge_tests_handler.py:45: multiple statements on one line (semicolon) [pep8/E702] [5]
Total errors found: 1 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
The style error is caused by ";" in content-type http header. Unfortunately, we use a third-party script for python style check so it's non-trivial to fix it. Ping reviewers. This patch is blocking my patch to add memcache. Comment on attachment 124388 [details]
Adds admin/merge-tests
I'm not sure what sort of review you're looking for here. Generally, we use PEP8 style in our python (but without the 80 col limit). Names like mergedResults don't match that style, but given that this is an addition to code imported from Mozilla, we should probably match their style. Are you planning to contribute these changes upstream?
In any case, I'm inclined to rubber-stamp these sorts of changes.
Thanks for the review. (In reply to comment #5) > (From update of attachment 124388 [details]) > I'm not sure what sort of review you're looking for here. Generally, we use PEP8 style in our python (but without the 80 col limit). Names like mergedResults don't match that style, but given that this is an addition to code imported from Mozilla, we should probably match their style. They use names with underscores? I can change them since all the python code I'm checking into webkit won't be shared with Mozilla. > Are you planning to contribute these changes upstream? No. Python backend is specific to WebKit. In fact, all patches I've contributed so far only contains WebKit-specific code. As I've outlined in http://trac.webkit.org/changeset/105971, we need to import some code from https://github.com/mozilla/graphs but it's all in front-end (css, js, html). > They use names with underscores? I can change them since all the python code I'm checking into webkit won't be shared with Mozilla.
Yeah. I'm surprised check-webkit-style didn't complain. (Maybe it stops at the first error in each file?)
Comment on attachment 124388 [details] Adds admin/merge-tests Clearing flags on attachment: 124388 Committed r106292: <http://trac.webkit.org/changeset/106292> All reviewed patches have been landed. Closing bug. |