|Summary:||[check-webkit-style] False positive for tst_QWebFrame|
|Product:||WebKit||Reporter:||Adam Barth <abarth>|
|Component:||Tools / Tests||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||benjamin, commit-queue, eric, levin, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Adam Barth 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]  I think tst_QWebFrame is a fine name in this test system.
Comment 2 WebKit Review Bot 2009-12-11 09:42:20 PST
style-queue ran check-webkit-style on attachment 44693 [details] without any errors.
Comment 3 Eric Seidel (no email) 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. ;)
Comment 4 Benjamin Poulain 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.
Comment 5 David Levin 2009-12-11 11:11:18 PST
Consider using "startswith" instead of find() == 0.
Comment 6 Adam Barth 2009-12-11 12:41:37 PST
Created attachment 44702 [details] Updated with reviewer comments
Comment 7 Adam Barth 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.
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2009-12-11 12:51:00 PST
All reviewed patches have been landed. Closing bug.