[webkit-reviews] review denied: [Bug 17366] ContextMenu::itemAtIndex() should be static and declared for Windows port only : [Attachment 19127] the patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 20 09:30:30 PST 2008


Darin Adler <darin at apple.com> has denied Oleg Sukhodolsky
<oleg.sukhodolsky at sun.com>'s request for review:
Bug 17366: ContextMenu::itemAtIndex() should be static and declared for Windows
port only
http://bugs.webkit.org/show_bug.cgi?id=17366

Attachment 19127: the patch
http://bugs.webkit.org/attachment.cgi?id=19127&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'd prefer to not have the #if in the header. There's no harm in declaring a
function that isn't defined on all platforms, especially if we might want to
define and use it on other platforms in the future. Is there something I'm
overlooking here?

+	 static ContextMenuItem* itemAtIndex(unsigned, const
PlatformMenuDescription);

The "const" here, while not new, is not helpful. It doesn't have any effect on
the type of the parameter.

Since there's no significant benefit to this patch, I'm going to set it review-
just because of the minor comments above. If there was a clearer benefit I
might just overlook these and say review+.


More information about the webkit-reviews mailing list