[webkit-reviews] review granted: [Bug 105735] Move ENABLE macros for WebCore out from Platform.h and add all the macros from the FeatureFlags wiki : [Attachment 180718] Fix style.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 16 15:22:18 PST 2013


Darin Adler <darin at apple.com> has granted Laszlo Gombos
<laszlo.gombos at webkit.org>'s request for review:
Bug 105735: Move ENABLE macros for WebCore out from Platform.h and add all the
macros from the FeatureFlags wiki
https://bugs.webkit.org/show_bug.cgi?id=105735

Attachment 180718: Fix style.
https://bugs.webkit.org/attachment.cgi?id=180718&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=180718&action=review


Seems like a step in the right direction. We do want to have the feature
definition system separate from the platform detection system. What I had
imagined was that each platform would have its own file listing every feature
that it wanted to be different than the default, rather than having one central
file with #if statements in it.

I don’t think that having a strict rule that every ENABLE has to be in
FeatureDefines.h is quite right, though. It seems especially bad to move
ENABLE_DEBUG_MATH_LAYOUT. Maybe we don’t want to use ENABLE() with that, but we
should not pollute FeatureDefines.h with it.

> Source/WTF/wtf/FeatureDefines.h:56
> +/* ENABLE macro defaults for WebCore */

Something should say that these are in alphabetical order.

> Source/WTF/wtf/FeatureDefines.h:66
> +/* Add other platforms as they update their platfrom specific code to handle
TextRun's with 8 bit data. */

Typo: platfrom


More information about the webkit-reviews mailing list