[webkit-reviews] review granted: [Bug 77118] [Qt][Mac] Build fails after adding ICU support (r105997). : [Attachment 124300] patch for review. - rebased and fixed indentation.

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has granted Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 77118: [Qt][Mac] Build fails after adding ICU support (r105997).
https://bugs.webkit.org/show_bug.cgi?id=77118

Attachment 124300: patch for review. - rebased and fixed indentation.
https://bugs.webkit.org/attachment.cgi?id=124300&action=review

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=124300&action=review


r=me with some comments sprinkled around and tiny fix

> Source/WTF/WTF.pri:22
> +	   INCLUDEPATH += $${ROOT_WEBKIT_DIR}/Source/WTF/icu

Perhaps a comment here about why we're getting the headers from the webkit
sources, but the lib from the system.

> Source/WTF/WTF.pri:29
> +	       haveQt(5): error("To build QtWebKit with Qt 5 you need ICU")

Already in haveQt(5) scope, no need for this one

> Source/WebCore/Target.pri:2874
> +	   SOURCES += editing/SmartReplaceCF.cpp

Perhaps a comment on why we use the CF one on mac (that it matches what the mac
port does)

> Tools/qmake/mkspecs/features/features.prf:33
> +haveQt(5):if(contains(QT_CONFIG,icu)|mac) {

Perhaps a comment on why we don't rely on QT_CONFIG for mac (since it's not an
official support for all of qt)