[webkit-reviews] review denied: [Bug 16669] autotools update and fixes : [Attachment 18171] various autotools update and fixes patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jan Alonzo <jmalonzo at gmail.com>'s
request for review:
Bug 16669: autotools update and fixes
http://bugs.webkit.org/show_bug.cgi?id=16669

Attachment 18171: various autotools update and fixes patch
http://bugs.webkit.org/attachment.cgi?id=18171&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Although it's not new to this patch, I think it's a seriously bad idea that
GNUMakeFile.am has a copy of the contents of DerivedSources.make -- the whole
idea of DerivedSources.make is to have a shared makefile for these more
complicated rules. Surely we can find a way to incorporate those rules without
copying and pasting the entire file.

+#if ENABLE(DATABASE)
+#include "Database.h"
+#endif

This change is wrong. The file "Database.h" already has an #if ENABLE(DATABASE)
around its contents. We don't want to ifdef it in both places.

+#if ENABLE(DATABASE)
+#include "DatabaseTracker.h"
+#endif

We need to be consistent. The ENABLE #if statements are inside Database.h and
should go inside DatabaseTracker.h too. Or we could go the other way and change
how we handle optional things like database.

ENABLE_XBL should not be in the makefile. XBL was an experiment done more than
two years ago. It doesn't make sense to have it commented-out code in there
with a FIXME.

Everything else looks fine.

review- because of the database include files issue.