[webkit-reviews] review requested: [Bug 9179] Implement
select.options.add() method : [Attachment 9339] Patch v4
bugzilla-request-daemon at opendarwin.org
bugzilla-request-daemon at opendarwin.org
Mon Jul 10 07:34:52 PDT 2006
David Kilzer (ddkilzer) <ddkilzer at kilzer.net> has asked for review:
Bug 9179: Implement select.options.add() method
http://bugzilla.opendarwin.org/show_bug.cgi?id=9179
Attachment 9339: Patch v4
http://bugzilla.opendarwin.org/attachment.cgi?id=9339&action=edit
------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at kilzer.net>
Changes since Patch v3:
(In reply to comment #27)
> (From update of attachment 9325 [edit])
> 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 hate duplicating code. I made the one-argument method inline in value.h. Is
that okay?
> I think the bool& parameter's purpose is not self-explanatory and so I think
it
> should have a name in the header.
Added.
> I don't see the code to set $maybeOkParam!
This value is set from the return value of TypeCanFailConversion(), which is
the line of code in question below.
> + return 1; # 0; # or can it?
>
> The above line of code is confusing, even more than it was before.
Sorry...I left the old code in when first attempting to change the value. I
removed "# 0; # or can it?" in this patch.
More information about the webkit-reviews
mailing list