[webkit-reviews] review denied: [Bug 11617] Compile and work on
Qt/KDE platforms : [Attachment 11539] fixes the style issues
bugzilla-request-daemon at macosforge.org
bugzilla-request-daemon at macosforge.org
Thu Nov 16 14:14:48 PST 2006
mitz at webkit.org has denied mitz at webkit.org's request for review:
Bug 11617: Compile and work on Qt/KDE platforms
http://bugs.webkit.org/show_bug.cgi?id=11617
Attachment 11539: fixes the style issues
http://bugs.webkit.org/attachment.cgi?id=11539&action=edit
------- Additional Comments from mitz at webkit.org
r- since there are still a few coding style and comment issues, but other than
that the patch looks good.
Please avoid trailing spaces:
- ${CMAKE_CURRENT_SOURCE_DIR}/page
+ ${CMAKE_CURRENT_SOURCE_DIR}/page
Don't use tabs in ChangeLog:
+ http://bugs.webkit.org/show_bug.cgi?id=11617
+ Adjusting to the new api.
I don't think this comment makes sense for Qt:
+ // Double-click events don't exist in Cocoa. Since
passWidgetMouseDownEventToWidget will
+ // just pass currentEvent down to the widget, we don't want to call it for
events that
+ // don't correspond to Cocoa events. The mousedown/ups will have already
been passed on as
+ // part of the pressed/released handling.
What's the point of this comment?
+ // FIXME: this method always returns true
+ notImplemented();
+ return false;
+}
This should be '#include "Shared.h"'
+#include <Shared.h>
Should use braces for one-liners:
+ if (!m_menu) {
+ m_menu = new QMenu();
+ }
FIXME style is "// FIXME: Sentence-capitalized comment". I think these comments
could be phrased better (the second one might not make sense once the first one
is changed or removed):
+ //FIXME this method is silly
+ //FIXME another silly method
Include <WTF/Forward.h> and drop the WTF:: from these:
+ virtual void
registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand>);
+ virtual void
registerCommandForRedo(WTF::PassRefPtr<WebCore::EditCommand>);
More information about the webkit-reviews
mailing list