[webkit-reviews] review denied: [Bug 35844] [Qt] Add support for LayoutTestController commands. : [Attachment 52417] Removed stray changelog entry

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 2 11:40:40 PDT 2010


Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Robert Hogan
<robert at webkit.org>'s request for review:
Bug 35844: [Qt] Add support for LayoutTestController commands.
https://bugs.webkit.org/show_bug.cgi?id=35844

Attachment 52417: Removed stray changelog entry
https://bugs.webkit.org/attachment.cgi?id=52417&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
As it is an internal class we should call it QtDRTSupport (or QtDrtSupport to
following Qt naming conventions), as we do with most other internal Qt classes
(not privates) in WebKit.

We still need to look at the method names. For instance it is not obvious what
QtDRTSupport::run() does. Also, I would like you to group the methods logically
together in the header. Also, you have methods in the header with signature
(QWebFrame* qFrame, ...), please leave our the qFrame as the style guide
dictates.


More information about the webkit-reviews mailing list