[Webkit-unassigned] [Bug 27425] adding GDOMHTMLElementWrapperFactory[.cpp/.h]

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27425


Adam Treat <treat at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33052|review?                     |review-
               Flag|                            |




--- Comment #2 from Adam Treat <treat at kde.org>  2009-07-20 19:51:39 PDT ---
(From update of attachment 33052)
> +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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.