[webkit-reviews] review denied: [Bug 36761] [EFL] Modify the autotools build system to add support to build the EFL port : [Attachment 52343] Changes to the autotools build system to build the EFL port

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Leandro Pereira
<leandro at profusion.mobi>'s request for review:
Bug 36761: [EFL] Modify the autotools build system to add support to build the
EFL port
https://bugs.webkit.org/show_bug.cgi?id=36761

Attachment 52343: Changes to the autotools build system to build the EFL port
https://bugs.webkit.org/attachment.cgi?id=52343&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
 228 libJavaScriptCore_la_LIBADD += \
 229	$(ECORE_X_LIBS) \
 230	$(EFLDEPS_LIBS)

This sounds like a bad idea. It's your decision to make, but I think you should
try to keep X-related deps out of JSC.

20022032 if TARGET_X11
 2033 if GTK_PORT
20032034 webcoregtk_sources += \
20042035	WebCore/plugins/gtk/gtk2xtbin.c \
20052036	WebCore/plugins/gtk/gtk2xtbin.h \
20062037	WebCore/plugins/gtk/xembed.h
20072038 endif
 2039 endif

Perhaps GTK_PORT should be the outer check, instead? Are you also going to use
TARGET_*?

 28 # Benchmark tools ######################################################
 29 Programs_EWebBenchStress_CPPFLAGS = \
 30	-I$(top_srcdir)/WebKit/efl \
 31	-I$(top_srcdir)/WebKit/efl/ewk \
 32	$(EFLDEPS_CFLAGS)

Looks like these are no longer going to go in? This doesn't look too bad,
actually. I was expecting more surgery. I would like to get xan at least to
look at this before it goes in, given the importance of the build system. It
does get more complicated, but that may just be the kick we need to switch to
something better =P. r- for now because of the comments above that need
addressing.