[webkit-reviews] review denied: [Bug 9179] Implement select.options.add() method : [Attachment 9325] Patch v3

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Jul 9 23:32:56 PDT 2006


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 9179: Implement select.options.add() method
http://bugzilla.opendarwin.org/show_bug.cgi?id=9179

Attachment 9325: Patch v3
http://bugzilla.opendarwin.org/attachment.cgi?id=9325&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This seems nearly perfect even though I have a few minor quibbles.

I'm concerned about adding another level of function call for every caller of
toInt32, which might be hot. Maybe just repeat the code twice?

I think the bool& parameter's purpose is not self-explanatory and so I think it
should have a name in the header.

I don't see the code to set $maybeOkParam!

+    return 1; # 0; # or can it?

The above line of code is confusing, even more than it was before.

This is getting really close, but review- for now for at least some of these
questions.

This change looks really great. Lets get it landed!



More information about the webkit-reviews mailing list