RESOLVED FIXED 98442
[BlackBerry] Sync up CMake files 
https://bugs.webkit.org/show_bug.cgi?id=98442
Summary [BlackBerry] Sync up CMake files
Rob Buis
Reported 2012-10-04 13:04:17 PDT
SSIA
Attachments
Patch (14.42 KB, patch)
2012-10-04 13:11 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (14.31 KB, patch)
2012-10-04 14:32 PDT, Rob Buis 		gyuyoung.kim: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2012-10-04 13:11:35 PDT
Created attachment 167161 [details] Patch
Raphael Kubo da Costa (:rakuco)
Comment 2 2012-10-04 13:45:57 PDT
Comment on attachment 167161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167161&action=review Random comments from a bystander below. > Source/WebKit/PlatformBlackBerry.cmake:7 > + "${JAVASCRIPTCORE_DIR}/.." Are you sure you need this? This is the same directory you are removing below at the end of this LIST(APPEND ...) call, ${CMAKE_SOURCE_DIR}/Source. Isn't ${JAVASCRIPTCORE_DIR}/ForwardingHeaders (or some generated JSC directory) enough for the JSC API? > Source/WebKit/PlatformBlackBerry.cmake:28 > - "${CMAKE_SOURCE_DIR}/Source" # For JavaScriptCore API headers > + "${CMAKE_SOURCE_DIR}" # For JavaScriptCore API headers The comment here does not make much sense after the change. > Source/WebKit/PlatformBlackBerry.cmake:34 > + LIST(APPEND WebKit_INCLUDE_DIRECTORIES > + "${WEBCORE_DIR}/Modules/notifications" > + ) The indentation looks wrong. > Source/WebKit/PlatformBlackBerry.cmake:245 > +ENDIF (NOT PUBLIC_BUILD) ENDIF () should be fine. > Source/WebKit/PlatformBlackBerry.cmake:251 > +ENDIF (ENABLE_VIDEO_TRACK) Ditto.
Rob Buis
Comment 3 2012-10-04 13:51:52 PDT
(In reply to comment #2) > (From update of attachment 167161 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167161&action=review > > Random comments from a bystander below. > > > Source/WebKit/PlatformBlackBerry.cmake:7 > > + "${JAVASCRIPTCORE_DIR}/.." > > Are you sure you need this? This is the same directory you are removing below at the end of this LIST(APPEND ...) call, ${CMAKE_SOURCE_DIR}/Source. Isn't ${JAVASCRIPTCORE_DIR}/ForwardingHeaders (or some generated JSC directory) enough for the JSC API? > > > Source/WebKit/PlatformBlackBerry.cmake:28 > > - "${CMAKE_SOURCE_DIR}/Source" # For JavaScriptCore API headers > > + "${CMAKE_SOURCE_DIR}" # For JavaScriptCore API headers > > The comment here does not make much sense after the change. > > > Source/WebKit/PlatformBlackBerry.cmake:34 > > + LIST(APPEND WebKit_INCLUDE_DIRECTORIES > > + "${WEBCORE_DIR}/Modules/notifications" > > + ) > > The indentation looks wrong. > > > Source/WebKit/PlatformBlackBerry.cmake:245 > > +ENDIF (NOT PUBLIC_BUILD) > > ENDIF () should be fine. > > > Source/WebKit/PlatformBlackBerry.cmake:251 > > +ENDIF (ENABLE_VIDEO_TRACK) > > Ditto. Seems to make sense, I'll make a new patch, thanks!
Rob Buis
Comment 4 2012-10-04 14:32:41 PDT
Created attachment 167176 [details] Patch
Raphael Kubo da Costa (:rakuco)
Comment 5 2012-10-04 14:39:54 PDT
Comment on attachment 167176 [details] Patch Looks fine from a build system PoV.
Gyuyoung Kim
Comment 6 2012-10-04 18:06:16 PDT
Comment on attachment 167176 [details] Patch Looks fine.
Rob Buis
Comment 7 2013-01-04 09:05:49 PST
This was landed in r130503, closing.
Note You need to log in before you can comment on or make changes to this bug.