Bug 32436

Summary: [check-webkit-style] False positive for tst_QWebFrame
Product: WebKit Reporter: Adam Barth <abarth>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, eric, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Updated with reviewer comments none

Adam Barth
Reported 2009-12-11 09:17:29 PST
https://bugs.webkit.org/show_bug.cgi?id=29008#c4 WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:2084: tst_QWebFrame::arrayObjectEnumerable is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] I think tst_QWebFrame is a fine name in this test system.
Attachments
Patch (2.29 KB, patch)
2009-12-11 09:39 PST, Adam Barth
no flags
Updated with reviewer comments (2.35 KB, patch)
2009-12-11 12:41 PST, Adam Barth
no flags
Adam Barth
Comment 1 2009-12-11 09:39:06 PST
WebKit Review Bot
Comment 2 2009-12-11 09:42:20 PST
style-queue ran check-webkit-style on attachment 44693 [details] without any errors.
Eric Seidel (no email)
Comment 3 2009-12-11 10:00:03 PST
Comment on attachment 44693 [details] Patch It wasn't initially clear to me from the ChangeLog or the change that this was related to function names. Perhaps the changelog text could be improved. You might also note that tst_ is a required name for the Qt unit testing system (I assume it's required?) because if it's optional it's not a very good name. ;)
Benjamin Poulain
Comment 4 2009-12-11 10:59:51 PST
Thanks for the patch! (In reply to comment #3) > You might also note that tst_ is a required name for the Qt unit testing system > (I assume it's required?) because if it's optional it's not a very good name. > ;) Yep, tst_ is mandatory for the files containing the test suite. The folder of a particular test can also contain utility files with classes/mockup used by the test suite.
David Levin
Comment 5 2009-12-11 11:11:18 PST
Consider using "startswith" instead of find() == 0.
Adam Barth
Comment 6 2009-12-11 12:41:37 PST
Created attachment 44702 [details] Updated with reviewer comments
Adam Barth
Comment 7 2009-12-11 12:43:56 PST
(In reply to comment #5) > Consider using "startswith" instead of find() == 0. Thanks! I don't actually know python, so suggestions like this are much appreciated.
WebKit Commit Bot
Comment 8 2009-12-11 12:50:55 PST
Comment on attachment 44702 [details] Updated with reviewer comments Clearing flags on attachment: 44702 Committed r52016: <http://trac.webkit.org/changeset/52016>
WebKit Commit Bot
Comment 9 2009-12-11 12:51:00 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.