32436 2009-12-11 09:17:29 -0800 [check-webkit-style] False positive for tst_QWebFrame 2009-12-11 12:51:00 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 abarth webkit-unassigned benjamin commit-queue eric levin webkit.review.bot oldest_to_newest 170781 0 abarth 2009-12-11 09:17:29 -0800 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. 170790 1 44693 abarth 2009-12-11 09:39:06 -0800 Created attachment 44693 Patch 170792 2 webkit.review.bot 2009-12-11 09:42:20 -0800 style-queue ran check-webkit-style on attachment 44693 without any errors. 170796 3 44693 eric 2009-12-11 10:00:03 -0800 Comment on attachment 44693 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. ;) 170836 4 benjamin 2009-12-11 10:59:51 -0800 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. 170842 5 levin 2009-12-11 11:11:18 -0800 Consider using "startswith" instead of find() == 0. 170873 6 44702 abarth 2009-12-11 12:41:37 -0800 Created attachment 44702 Updated with reviewer comments 170874 7 abarth 2009-12-11 12:43:56 -0800 (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. 170879 8 44702 commit-queue 2009-12-11 12:50:55 -0800 Comment on attachment 44702 Updated with reviewer comments Clearing flags on attachment: 44702 Committed r52016: <http://trac.webkit.org/changeset/52016> 170880 9 commit-queue 2009-12-11 12:51:00 -0800 All reviewed patches have been landed. Closing bug. 44693 2009-12-11 09:39:06 -0800 2009-12-11 12:41:32 -0800 Patch bug-32436-20091211093904.patch text/plain 2344 abarth ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCAxMTlkMjY1Li4yMjE5MmI4IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDktMTIt MTEgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbY2hlY2std2Via2l0LXN0eWxlXSBGYWxzZSBw b3NpdGl2ZSBmb3IgdHN0X1FXZWJGcmFtZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MzI0MzYKKworICAgICAgICBBZGQgYW4gZXhjZXB0aW9uIGZvciBp ZGVudGlmaWVycyBzdGFydGluZyB3aXRoICJ0c3RfIi4KKworICAgICAgICAqIFNjcmlwdHMvbW9k dWxlcy9jcHBfc3R5bGUucHk6CisgICAgICAgICogU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91 bml0dGVzdC5weToKKwogMjAwOS0xMi0xMSAgU2ltb24gSGF1c21hbm4gIDxoYXVzbWFubkB3ZWJr aXQub3JnPiwgS2ltIEdyw7ZuaG9sbSAgPGtpbS5ncm9uaG9sbUBub21vdm9rLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBBbnR0aSBLb2l2aXN0by4KZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xz L1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHkgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVs ZXMvY3BwX3N0eWxlLnB5CmluZGV4IGQ4Y2E4ZDEuLmY2MDk1ZWQgMTAwNjQ0Ci0tLSBhL1dlYktp dFRvb2xzL1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHkKKysrIGIvV2ViS2l0VG9vbHMvU2Ny aXB0cy9tb2R1bGVzL2NwcF9zdHlsZS5weQpAQCAtMjczNyw2ICsyNzM3LDcgQEAgZGVmIGNoZWNr X2lkZW50aWZpZXJfbmFtZV9pbl9kZWNsYXJhdGlvbihmaWxlbmFtZSwgbGluZV9udW1iZXIsIGxp bmUsIGVycm9yKToKICAgICAgICAgaWYgbW9kaWZpZWRfaWRlbnRpZmllci5maW5kKCdfJykgPj0g MDoKICAgICAgICAgICAgICMgVmFyaW91cyBleGNlcHRpb25zIHRvIHRoZSBydWxlOiBKYXZhU2Ny aXB0IG9wIGNvZGVzIGZ1bmN0aW9ucywgY29uc3RfaXRlcmF0b3IuCiAgICAgICAgICAgICBpZiAo bm90IChmaWxlbmFtZS5maW5kKCdKYXZhU2NyaXB0Q29yZScpID49IDAgYW5kIG1vZGlmaWVkX2lk ZW50aWZpZXIuZmluZCgnX29wXycpID49IDApCisgICAgICAgICAgICAgICAgYW5kIG5vdCBtb2Rp ZmllZF9pZGVudGlmaWVyLmZpbmQoJ3RzdF8nKSA9PSAwCiAgICAgICAgICAgICAgICAgYW5kIG5v dCBtb2RpZmllZF9pZGVudGlmaWVyID09ICJjb25zdF9pdGVyYXRvciIpOgogICAgICAgICAgICAg ICAgIGVycm9yKGZpbGVuYW1lLCBsaW5lX251bWJlciwgJ3JlYWRhYmlsaXR5L25hbWluZycsIDQs IGlkZW50aWZpZXIgKyAiIGlzIGluY29ycmVjdGx5IG5hbWVkLiBEb24ndCB1c2UgdW5kZXJzY29y ZXMgaW4geW91ciBpZGVudGlmaWVyIG5hbWVzLiIpCiAKZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xz L1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGVfdW5pdHRlc3QucHkgYi9XZWJLaXRUb29scy9TY3Jp cHRzL21vZHVsZXMvY3BwX3N0eWxlX3VuaXR0ZXN0LnB5CmluZGV4IDc1ZGQ0N2UuLjQxYzg4OGMg MTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGVfdW5pdHRl c3QucHkKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91bml0dGVz dC5weQpAQCAtMzcwMyw2ICszNzAzLDkgQEAgY2xhc3MgV2ViS2l0U3R5bGVUZXN0KENwcFN0eWxl VGVzdEJhc2UpOgogICAgICAgICBzZWxmLmFzc2VydF9saW50KCd2b2lkIHRoaXNfb3BfY29kZShp bnQgdmFyMSwgaW50IHZhcjIpJywgJycsICdKYXZhU2NyaXB0Q29yZS9mb28uY3BwJykKICAgICAg ICAgc2VsZi5hc3NlcnRfbGludCgndm9pZCB0aGlzX29wX2NvZGUoaW50IHZhcjEsIGludCB2YXIy KScsICd0aGlzX29wX2NvZGUnICsgbmFtZV9lcnJvcl9tZXNzYWdlKQogCisgICAgICAgICMgVGhl cmUgaXMgYW4gZXhjZXB0aW9uIGZvciBzb21lIHVuaXQgdGVzdHMgdGhhdCBiZWdpbiB3aXRoICJ0 c3RfIi4KKyAgICAgICAgc2VsZi5hc3NlcnRfbGludCgndm9pZCB0c3RfUVdlYkZyYW1lOjphcnJh eU9iamVjdEVudW1lcmFibGUoaW50IHZhcjEsIGludCB2YXIyKScsICcnKQorCiAgICAgICAgICMg Y29uc3RfaXRlcmF0b3IgaXMgYWxsb3dlZCBhcyB3ZWxsLgogICAgICAgICBzZWxmLmFzc2VydF9s aW50KCd0eXBlZGVmIFZlY3RvclR5cGU6OmNvbnN0X2l0ZXJhdG9yIGNvbnN0X2l0ZXJhdG9yOycs ICcnKQogCg== 44702 2009-12-11 12:41:37 -0800 2009-12-11 12:50:55 -0800 Updated with reviewer comments bug-32436-20091211124136.patch text/plain 2406 abarth ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCAxMTlkMjY1Li45NjI1ZmIxIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMDktMTIt MTEgIEFkYW0gQmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBi eSBFcmljIFNlaWRlbC4KKworICAgICAgICBbY2hlY2std2Via2l0LXN0eWxlXSBGYWxzZSBwb3Np dGl2ZSBmb3IgdHN0X1FXZWJGcmFtZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MzI0MzYKKworICAgICAgICBBZGQgYW4gZXhjZXB0aW9uIGZvciBmdW5j dGlvbiBuYW1lcyB0aGF0IHN0YXJ0IHdpdGggInRzdF8iLiAgVGhlc2UgYXJlCisgICAgICAgIHVz ZWQgYnkgdGhlIFF0IHVuaXQgdGVzdGluZyBmcmFtZXdvcmsuCisKKyAgICAgICAgKiBTY3JpcHRz L21vZHVsZXMvY3BwX3N0eWxlLnB5OgorICAgICAgICAqIFNjcmlwdHMvbW9kdWxlcy9jcHBfc3R5 bGVfdW5pdHRlc3QucHk6CisKIDIwMDktMTItMTEgIFNpbW9uIEhhdXNtYW5uICA8aGF1c21hbm5A d2Via2l0Lm9yZz4sIEtpbSBHcsO2bmhvbG0gIDxraW0uZ3JvbmhvbG1Abm9tb3Zvay5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgQW50dGkgS29pdmlzdG8uCmRpZmYgLS1naXQgYS9XZWJLaXRU b29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlLnB5IGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9t b2R1bGVzL2NwcF9zdHlsZS5weQppbmRleCBkOGNhOGQxLi5mMjY4NjEwIDEwMDY0NAotLS0gYS9X ZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlLnB5CisrKyBiL1dlYktpdFRvb2xz L1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGUucHkKQEAgLTI3MzcsNiArMjczNyw3IEBAIGRlZiBj aGVja19pZGVudGlmaWVyX25hbWVfaW5fZGVjbGFyYXRpb24oZmlsZW5hbWUsIGxpbmVfbnVtYmVy LCBsaW5lLCBlcnJvcik6CiAgICAgICAgIGlmIG1vZGlmaWVkX2lkZW50aWZpZXIuZmluZCgnXycp ID49IDA6CiAgICAgICAgICAgICAjIFZhcmlvdXMgZXhjZXB0aW9ucyB0byB0aGUgcnVsZTogSmF2 YVNjcmlwdCBvcCBjb2RlcyBmdW5jdGlvbnMsIGNvbnN0X2l0ZXJhdG9yLgogICAgICAgICAgICAg aWYgKG5vdCAoZmlsZW5hbWUuZmluZCgnSmF2YVNjcmlwdENvcmUnKSA+PSAwIGFuZCBtb2RpZmll ZF9pZGVudGlmaWVyLmZpbmQoJ19vcF8nKSA+PSAwKQorICAgICAgICAgICAgICAgIGFuZCBub3Qg bW9kaWZpZWRfaWRlbnRpZmllci5zdGFydHN3aXRoKCd0c3RfJykKICAgICAgICAgICAgICAgICBh bmQgbm90IG1vZGlmaWVkX2lkZW50aWZpZXIgPT0gImNvbnN0X2l0ZXJhdG9yIik6CiAgICAgICAg ICAgICAgICAgZXJyb3IoZmlsZW5hbWUsIGxpbmVfbnVtYmVyLCAncmVhZGFiaWxpdHkvbmFtaW5n JywgNCwgaWRlbnRpZmllciArICIgaXMgaW5jb3JyZWN0bHkgbmFtZWQuIERvbid0IHVzZSB1bmRl cnNjb3JlcyBpbiB5b3VyIGlkZW50aWZpZXIgbmFtZXMuIikKIApkaWZmIC0tZ2l0IGEvV2ViS2l0 VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91bml0dGVzdC5weSBiL1dlYktpdFRvb2xz L1NjcmlwdHMvbW9kdWxlcy9jcHBfc3R5bGVfdW5pdHRlc3QucHkKaW5kZXggNzVkZDQ3ZS4uNDFj ODg4YyAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvU2NyaXB0cy9tb2R1bGVzL2NwcF9zdHlsZV91 bml0dGVzdC5weQorKysgYi9XZWJLaXRUb29scy9TY3JpcHRzL21vZHVsZXMvY3BwX3N0eWxlX3Vu aXR0ZXN0LnB5CkBAIC0zNzAzLDYgKzM3MDMsOSBAQCBjbGFzcyBXZWJLaXRTdHlsZVRlc3QoQ3Bw U3R5bGVUZXN0QmFzZSk6CiAgICAgICAgIHNlbGYuYXNzZXJ0X2xpbnQoJ3ZvaWQgdGhpc19vcF9j b2RlKGludCB2YXIxLCBpbnQgdmFyMiknLCAnJywgJ0phdmFTY3JpcHRDb3JlL2Zvby5jcHAnKQog ICAgICAgICBzZWxmLmFzc2VydF9saW50KCd2b2lkIHRoaXNfb3BfY29kZShpbnQgdmFyMSwgaW50 IHZhcjIpJywgJ3RoaXNfb3BfY29kZScgKyBuYW1lX2Vycm9yX21lc3NhZ2UpCiAKKyAgICAgICAg IyBUaGVyZSBpcyBhbiBleGNlcHRpb24gZm9yIHNvbWUgdW5pdCB0ZXN0cyB0aGF0IGJlZ2luIHdp dGggInRzdF8iLgorICAgICAgICBzZWxmLmFzc2VydF9saW50KCd2b2lkIHRzdF9RV2ViRnJhbWU6 OmFycmF5T2JqZWN0RW51bWVyYWJsZShpbnQgdmFyMSwgaW50IHZhcjIpJywgJycpCisKICAgICAg ICAgIyBjb25zdF9pdGVyYXRvciBpcyBhbGxvd2VkIGFzIHdlbGwuCiAgICAgICAgIHNlbGYuYXNz ZXJ0X2xpbnQoJ3R5cGVkZWYgVmVjdG9yVHlwZTo6Y29uc3RfaXRlcmF0b3IgY29uc3RfaXRlcmF0 b3I7JywgJycpCiAK