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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Dec 29 14:22:36 PST 2007

Darin Adler <darin at apple.com> has denied Jan Alonzo <jmalonzo at gmail.com>'s
request for review:
Bug 16669: autotools update and fixes

Attachment 18171: various autotools update and fixes patch

------- 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.

+#include "Database.h"

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.

+#include "DatabaseTracker.h"

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.

More information about the webkit-reviews mailing list