[webkit-reviews] review denied: [Bug 42466] Explicitly declare DeviceOrientationEvent destructor and define it in the .cpp file : [Attachment 61821] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jul 16 10:27:16 PDT 2010
David Levin <levin at chromium.org> has denied hans at chromium.org's request for
review:
Bug 42466: Explicitly declare DeviceOrientationEvent destructor and define it
in the .cpp file
https://bugs.webkit.org/show_bug.cgi?id=42466
Attachment 61821: Patch
https://bugs.webkit.org/attachment.cgi?id=61821&action=review
------- Additional Comments from David Levin <levin at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index
4558db60cf53f1b2d7bdc4fddb487c8516f91d66..079c9321a32ab8ba68b3b37fb35ed13fade05
33a 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,21 @@
> +2010-07-16 Hans Wennborg <hans at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Explicitly declare DeviceOrientationEvent destructor and define it
in the .cpp file
> + https://bugs.webkit.org/show_bug.cgi?id=42466
> +
> + This avoids needing to have the full declaration of
DeviceOrientation
> + in DeviceOrientationEvent.h, which was otherwise necessary because
> + of the implicit destructor.
Consider moving this comment (and shortening) to be where the function is
listed in the ChangeLog.
> + (Original problem at https://bugs.webkit.org/show_bug.cgi?id=42447)
> +
> + No new tests. (OOPS!)
This should be removed -- perhaps replaced by "No new functionality so no new
tests.".
> +
> + * dom/DeviceOrientationEvent.cpp:
> + (WebCore::DeviceOrientationEvent::~DeviceOrientationEvent):
Right here. Something like:
"Move here to voids needing to have the full declaration
of DeviceOrientation in DeviceOrientationEvent.h"
> + * dom/DeviceOrientationEvent.h:
r- due to OOPS for no new tests (as this won't auto-commit).
More information about the webkit-reviews
mailing list