[Qt] ChromeClientQt.h has coding-style errors
Created attachment 58050 [details] Patch
Comment on attachment 58050 [details] Patch WebKit/qt/WebCoreSupport/ChromeClientQt.h:36 + #include "RefCounted.h" #include <wtf/RefCounted.h>
Created attachment 87656 [details] ChromeClientQt.h style fixes It's been a long time since the original patch was submitted so I created a new patch.
Attachment 87656 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1 Source/WebKit/qt/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #4) > Attachment 87656 [details] did not pass style-queue: > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1 > Source/WebKit/qt/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] > Total errors found: 1 in 2 files > If any of these errors are false positives, please file a bug against check-webkit-style. There is a bug number in the Changelog so I'm not sure what is missing here.
Comment on attachment 87656 [details] ChromeClientQt.h style fixes View in context: https://bugs.webkit.org/attachment.cgi?id=87656&action=review > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:37 > +#include "RefCounted.h" That should be #include <wtf/RefCounted.h> > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:100 > + virtual void addMessageToConsole(MessageSource, MessageType, MessageLevel, const String& message, > + unsigned int lineNumber, const String& sourceID); You can put this on a single line, we don't limit line to 80 characters in WebKit. > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:152 > + // see ChromeClient.h You can remove that line. > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:153 > + // this is a hook for WebCore to tell us what we need to do with the GraphicsLayers Comments should be sentences, with a upper case at start, and a dot at the end. > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:200 > + WebCore::KURL lastHoverURL; > + WTF::String lastHoverTitle; > + WTF::String lastHoverContent; Where are those prefixed by the namespace? Aren't we in the WebCore namespace already? And from the API is looks like WTF is also defined.
Created attachment 87717 [details] New patch to fix ChromeClientQt.h style issues I have uploaded a new patch based on the comments. I still get style error that the Changelog does not have a bug number but it does so I'm not sure what to do about that.
Attachment 87717 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/ChangeLog', u'Source/WebK..." exit_code: 1 Source/WebKit/qt/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 87717 [details] New patch to fix ChromeClientQt.h style issues View in context: https://bugs.webkit.org/attachment.cgi?id=87717&action=review Looks good. Just check the namespace question before pushing. > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.h:113 > + virtual WebCore::KeyboardUIMode keyboardUIMode(); Is the WebCore namespace needed here?
(In reply to comment #7) > I have uploaded a new patch based on the comments. I still get style error that the Changelog does not have a bug number but it does so I'm not sure what to do about that. Check if there a character that should not be here in the checklog. It could be a \t or an non-breaking space in there.
Created attachment 87724 [details] New patch to fix Changelog style issue and namespace issue. I uploaded a new patch that fixes the Changelog style issue and namespace issue.
The commit-queue encountered the following flaky tests while processing attachment 87724 [details]: animations/suspend-resume-animation.html bug 48161 (author: cmarrin@apple.com) The commit-queue is continuing to process your patch.
Comment on attachment 87724 [details] New patch to fix Changelog style issue and namespace issue. Clearing flags on attachment: 87724 Committed r82585: <http://trac.webkit.org/changeset/82585>
All reviewed patches have been landed. Closing bug.