Summary: | [BlackBerry] Introduce a JavaScript Variant object | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Meyer <ben> | ||||||||||
Component: | WebKit BlackBerry | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | gyuyoung.kim, mifenton, rakuco, tonikitoo, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | Other | ||||||||||||
Attachments: |
|
Description
Benjamin Meyer
2012-08-09 13:12:22 PDT
Created attachment 157524 [details]
patch
Attachment 157524 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/ChangeLog', u'Source/WebKit/..." exit_code: 1
Source/WebKit/blackberry/Api/WebPage.h:24: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 157529 [details]
patch
Fixed style error
Comment on attachment 157529 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=157529&action=review I think this looks good overall. Please fix the issues and then I'll look one last time. > Source/WebKit/blackberry/Api/JavaScriptVariant.cpp:87 > + // Output warning? ASSERT_NOT_REACHED() I would say. > Source/WebKit/blackberry/Api/JavaScriptVariant.h:29 > + Remove the extra new line here. Created attachment 157551 [details]
patch
Corrections from Adam's review
Comment on attachment 157551 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=157551&action=review > Source/WebKit/blackberry/Api/JavaScriptVariant.h:31 > +class BLACKBERRY_EXPORT JavaScriptVariant { > + > +public: nit: extra blank line. > Source/WebKit/blackberry/ChangeLog:16 > + Reviewed by Joe Mason Should be "Internally reviewed by Joe" Created attachment 157771 [details]
patch
Changes from Antonio Gomes comments, (remove blank link and change review by link in the changelogs and commit message)
Comment on attachment 157771 [details] patch Clearing flags on attachment: 157771 Committed r125324: <http://trac.webkit.org/changeset/125324> All reviewed patches have been landed. Closing bug. |