[webkit-reviews] review denied: [Bug 63376] [DOM] Core part patch for supporting battery status event : [Attachment 98651] Add core patch files for the battery status event

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 27 10:08:38 PDT 2011


Leandro Pereira <leandro at profusion.mobi> has denied Kihong Kwon
<kihong.kwon at samsung.com>'s request for review:
Bug 63376: [DOM] Core part patch for supporting battery status event
https://bugs.webkit.org/show_bug.cgi?id=63376

Attachment 98651: Add core patch files for the battery status event
https://bugs.webkit.org/attachment.cgi?id=98651&action=review

------- Additional Comments from Leandro Pereira <leandro at profusion.mobi>
View in context: https://bugs.webkit.org/attachment.cgi?id=98651&action=review

Looks OK, but ChangeLog entry needs work.

> Source/WebCore/ChangeLog:6
> +	   [DOM] Core part patch for supporting battery status event
> +	   https://bugs.webkit.org/show_bug.cgi?id=63376

"Add JavaScript wrappers for battery status events." would read better IMO.
Also, things in brackets are usually used for port names. Since this is isn't
port-specific, you can remove it.

> Source/WebCore/ChangeLog:14
> +	   Add Core part modification for battery status event
> +	   - JSEventCustom.cpp, V8EventCustom.cpp
> +	     binding event between JavaSctipt and WebCore
> +	   - dom/Document.cpp Event.cpp Event.h Eventnames.h
> +	     Define and create event.
> +	   - Page.h Page.cpp, DOMWindow.h DOMWindow.cpp
> +	     Add/Remove event listener and init event handler to pageClient

These should go below, right after each file name.

> Source/WebCore/ChangeLog:18
> +	   no new tests.
> +	   This event implementation is not finished with this patch.
> +	   After finishing inplemetation, It will be added.

Begin sentences with a capital letter. Better yet, rephrase this to say
something like "Test cases will be added as soon as the whole battery status
feature is implemented.".


More information about the webkit-reviews mailing list