[webkit-reviews] review denied: [Bug 27425] adding GDOMHTMLElementWrapperFactory[.cpp/.h] : [Attachment 33052] adds gdomhtmlelementwrapperfactory.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 20 19:51:39 PDT 2009
Adam Treat <treat at kde.org> has denied Luke Kenneth Casson Leighton
<lkcl at lkcl.net>'s request for review:
Bug 27425: adding GDOMHTMLElementWrapperFactory[.cpp/.h]
https://bugs.webkit.org/show_bug.cgi?id=27425
Attachment 33052: adds gdomhtmlelementwrapperfactory.
https://bugs.webkit.org/attachment.cgi?id=33052&action=review
------- Additional Comments from Adam Treat <treat at kde.org>
> +2008-11-30 lkcl <lkcl at lkcl.net>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + WARNING: NO TEST CASES ADDED OR CHANGED
> +
> + https://bugs.webkit.org/show_bug.cgi?id=27425
> +
> + * bindings/gdom/GDOMHTMLElementWrapperFactory.cpp: Added. Copied
from the auto-generated JSHTMLElementWrapperFactory.cpp
> + * bindings/gdom/GDOMHTMLElementWrapperFactory.h: Added. Copied
from the auto-generated JSHTMLElementWrapperFactory.h
You are missing a description of what this patch actually does. Please fill
out some description for what this patch is meant to provide.
> +/* Note: This file is derived by hand from an automatically generated
> + file.
> +
> + Keeping it up-to-date could potentially be done by adding a
> + make_names.pl generator, or by writing a separate generater which
> + takes JSHTMLElementWrapperFactory.h as input.
> +*/
Minor nit: s/generater/generator/
Regular nit: why is this generated file to be landed? If it is to be updated
in the future with a script, then why land it in the tree as static? A
placeholder until said script is written?
> +
> +
> +#include "config.h"
> +
> +#include "gdom/GdomNode.h"
> +#include "GDOMHTMLElementWrapperFactory.h"
Order of includes is wrong. See the WebKit coding style guidelines for order
of includes.
There a lot of commented out pieces of code. We generally don't land patches
with commented out pieces of code.
Beyond that, without a more detailed Changelog it makes it hard to review.
r- for the above reasons.
More information about the webkit-reviews
mailing list