Summary: | [Qt] ChromeClientQt.h has coding-style errors | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Bakken <agbakken> | ||||||||||
Component: | New Bugs | Assignee: | Nancy Piedra <nancy.piedra> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Trivial | CC: | benjamin, commit-queue, jedrzej.nowacki, nancy.piedra, webkit.review.bot | ||||||||||
Priority: | P5 | Keywords: | EasyFix, Qt, QtTriaged | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Anders Bakken
2010-06-07 11:18:21 PDT
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. |