[webkit-reviews] review denied: [Bug 34994] Add sync bindings for Worker access to DB : [Attachment 55358] patch #2: IDL files + JSC and V8 bindings

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has denied  review:
Bug 34994: Add sync bindings for Worker access to DB
https://bugs.webkit.org/show_bug.cgi?id=34994

Attachment 55358: patch #2: IDL files + JSC and V8 bindings
https://bugs.webkit.org/attachment.cgi?id=55358&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>
I'd actually prefer a little more explanation and focus here before it lands
(yes, it's much better, and thanks for sticking to it)

The empty ChangeLog is not helpful.  New files might be self explanatory, but
I'd like to see comments on changes to existing code.

> -    RefPtr<SQLTransactionCallback>
callback(JSSQLTransactionCallback::create(object,
static_cast<JSDOMGlobalObject*>(exec->dynamicGlobalObject())));
> -    
> +
> +    RefPtr<SQLTransactionCallback>
callback(JSSQLTransactionCallback::create(object,
static_cast<JSDOMGlobalObject*>(globalObject())));

This replacement is done a lot in this patch.  Why this change?  There's no
comment in the ChangeLog explaining why it's necessary.  

If it's just a style cleanup because we have some new helper-y way of doing
this, that should be in a separate patch because it is unrelated to this task.