Summary: | Remove custom bindings form ScriptProfileNode.idl of attribute type Array. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Vineet Chaudhary (vineetc) <code.vineet> | ||||||
Component: | New Bugs | Assignee: | Vineet Chaudhary (vineetc) <code.vineet> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, haraken, japhet, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Vineet Chaudhary (vineetc)
2012-03-22 05:14:40 PDT
(In reply to comment #0) > Remove custom bindings for attribute type Array from ScriptProfileNode.idl. > Replace readonly attribute [CustomGetter] Array children; > To readonly attribute sequence<ProfileNode> children; Sorry for typo shouldBe readonly attribute sequence<ScriptProfileNode> children; Created attachment 133230 [details]
Patch
Patch for review.
Looks OK. Would you change r to r? if you want a review? Comment on attachment 133230 [details]
Patch
Thanks for quick review. :)
Comment on attachment 133230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133230&action=review > Source/WebCore/ChangeLog:8 > + No new tests. Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)". > Source/WebCore/ChangeLog:10 > + replace [CustomGetter] Array with sequence<ScriptProfileNode>. Normally, we write such explanations between the "Reviewed by" line and the "Test:" line. (In reply to comment #5) > (From update of attachment 133230 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133230&action=review > > > Source/WebCore/ChangeLog:8 > > + No new tests. > > Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)". You mean in LayoutTests? Sure I will do that. Also do I need to add this to TestObj.idl too? (In reply to comment #6) > > Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)". > > You mean in LayoutTests? Yes. The line will help people understand what your change is going to affect. > Also do I need to add this to TestObj.idl too? No:) The test is already added in the previous patch. (In reply to comment #5) > Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)". Just a clarification: I didn't mean to add a new test. I meant you can add a line that describes an existing test that might be affected by your change. Created attachment 133242 [details]
Updated Patch
Sorry for misunderstanding I was not aware of the new style in Changelogs.
Comment on attachment 133242 [details]
Updated Patch
OK. Thanks!
Comment on attachment 133242 [details] Updated Patch Clearing flags on attachment: 133242 Committed r111692: <http://trac.webkit.org/changeset/111692> All reviewed patches have been landed. Closing bug. |