[webkit-reviews] review denied: [Bug 28128] [Haiku] Modifications on WebCore. : [Attachment 34417] Patch to add tiny modifications on WebCore/platform/graphics files to allow Haiku port.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Aug 9 08:16:21 PDT 2009


Eric Seidel <eric at webkit.org> has denied Maxime Simon
<simon.maxime at gmail.com>'s request for review:
Bug 28128: [Haiku] Modifications on WebCore.
https://bugs.webkit.org/show_bug.cgi?id=28128

Attachment 34417: Patch to add tiny modifications on WebCore/platform/graphics
files to allow Haiku port.
https://bugs.webkit.org/attachment.cgi?id=34417&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Why virtual?
#if PLATFORM(HAIKU)
 175	 virtual BBitmap* getBitmap() const;
 176 #endif

Seems it should be createBBitmap()?  Or is it actually a getter?  in which case
it can be just bitmap(), no?

We generally don't use "get" in getters:
 388	     pattern	getHaikuStrokeStyle();

haikuStrokeStyle()

unless that's createing something in which case it should be create.  It looks
like it's just returning a stack object though, so it doesn't need "create" in
the name.

Why is this needed?
 #elif PLATFORM(HAIKU)
 82	Icon();
8183 #endif
please explain in the ChangeLog.

Seems like  a bad idea:
 #elif PLATFORM(HAIKU)
 127	 IntPoint(const BPoint&);
 128	 operator BPoint() const;
124129 #endif
You don't wan implicit conversion of floating point points to integer points.

Again, probably bad idea:
#elif PLATFORM(HAIKU)
 150	 IntRect(const BRect&);
 151	 operator BRect() const;
147152 #endif

Again:
 #if PLATFORM(HAIKU)
 119	 IntSize(const BSize&);
 120	 operator BSize() const;
 121 #endif

(this is all assuming that B* are floating point based).


More information about the webkit-reviews mailing list