[webkit-reviews] review denied: [Bug 20020] Proposed enhancement to JavaScriptCore API : [Attachment 22643] Add new API functions (JSObjectMake(Array|Date|Error|RegExp)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 4 19:28:19 PDT 2008


Sam Weinig <sam at webkit.org> has denied Kelvin Sherlock <ksherlock at gmail.com>'s
request for review:
Bug 20020: Proposed enhancement to JavaScriptCore API
https://bugs.webkit.org/show_bug.cgi?id=20020

Attachment 22643: Add new API functions (JSObjectMake(Array|Date|Error|RegExp)
https://bugs.webkit.org/attachment.cgi?id=22643&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
The * should be on the other side for JSValueRef *exception.  There is also an
extra space.
+JSObjectRef JSObjectMakeArray(JSContextRef ctx, size_t argumentCount, const
JSValueRef arguments[],  JSValueRef *exception)

+JSObjectRef JSObjectMakeDate(JSContextRef ctx, size_t argumentCount, const
JSValueRef arguments[],  JSValueRef *exception)

+JSObjectRef JSObjectMakeDate(JSContextRef ctx, size_t argumentCount, const
JSValueRef arguments[],  JSValueRef *exception)

+JSObjectRef JSObjectMakeRegExp(JSContextRef ctx, size_t argumentCount, const
JSValueRef arguments[],  JSValueRef *exception)

Same here and few other places.
+    JSObject *result;


We like to use ++i where we can.  This issue exists in a number of  places.
+	 for (size_t i = 0; i < argumentCount; i++)

The braces are not needed.
+    } else {
+	 result = constructEmptyArray(exec);
+    }

It is 2008!  :)
+ * Copyright (C) 2007 Kelvin W Sherlock (ksherlock at gmail.com)

There are some uses of tabs that need to be removed.

You should also add some tests to the testapi.c test.

r-


More information about the webkit-reviews mailing list