[webkit-reviews] review denied: [Bug 27734] WINCE PORT: font related changes : [Attachment 34178] remove unnecessary lines in FontCache.h
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 6 19:48:50 PDT 2009
Eric Seidel <eric at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 27734: WINCE PORT: font related changes
https://bugs.webkit.org/show_bug.cgi?id=27734
Attachment 34178: remove unnecessary lines in FontCache.h
https://bugs.webkit.org/attachment.cgi?id=34178&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
Please collapse this line into one nicely named define:
35 #if PLATFORM(CG) || PLATFORM(QT) || PLATFORM(GTK) || (PLATFORM(CHROMIUM) &&
PLATFORM(WIN_OS)) || PLATFORM(WINCE)
like STORE_FONT_POINTER or whatever the heck the ifdefed code does.
Please add more indetifying information than just "comment above":
179 // see comment above
otherwise if someone adds a comment between your two comments the latter
comment will be confusing.
// See comment about WINCE GDI handling near setGlyphDataForCharacter above.
or something.
Does WINCE only support horrizontal text?
3 #elif PLATFORM(WINCE)
64 typedef float GlyphBufferAdvance;
6065 #else
A comment is need there to explain why it's just a float.
No need for arg name:
51 FontCustomPlatformData* createFontCustomPlatformData(const
SharedBuffer* buffer);
Can you split this out?
r- mostly for the unexplained advances change.
More information about the webkit-reviews
mailing list