[webkit-reviews] review denied: [Bug 50861] [Qt][WK2] Make WebKit2 static library compile on Symbian : [Attachment 76385] Fixed patch with no WK2_DIR

[bugzilla-daemon at webkit.org]

Laszlo Gombos <laszlo.1.gombos at nokia.com> has denied Siddharth Mathur
<siddharth.mathur at nokia.com>'s request for review:
Bug 50861: [Qt][WK2] Make WebKit2 static library compile on Symbian
https://bugs.webkit.org/show_bug.cgi?id=50861

Attachment 76385: Fixed patch with no WK2_DIR
https://bugs.webkit.org/attachment.cgi?id=76385&action=review

------- Additional Comments from Laszlo Gombos <laszlo.1.gombos at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=76385&action=review

r- because of the unnecessary symbian guards and the expected maintenance
burden. Perhaps we should restrict this bug only to enable WebKit2 builds to
build without NPAPI support for the Qt port.

> WebKit2/WebKit2.pro:26
> +!symbian {

In general we should aim to minimize the symbian guards to keep the build
system manageable. I do not think this guard is essential - we have a similar
section in WebCore.pro without symbian guards. Is this section fixing something
?

> WebKit2/WebKit2.pro:38
> +    MMP_RULES += "USERINCLUDE ../WebCore/bridge"

There is a way around this problem by using the PREPEND_INCLUDEPATH variable.
r65877 is an example of how this was resolved for WebCore.pro earlier. 
I think we should either follow the WebCore.pro solution here as well or we
should delay addressing this problem after we got rid of the WebKit2 static
library; in fact the problem might go away after we got rid of WebKit2 library.


> WebKit2/WebKit2.pro:626
> +    !symbian: {

Let's re-use the "contains(DEFINES, ENABLE_NETSCAPE_PLUGIN_API=1)" guard here
the same way as we have in WebCore.pro; this makes the configuration available
on all QtWebKit ports and enables us to control this feature build time without
changing the source code (e.g. from build-webkit).