Bug 98442

Summary:

[BlackBerry] Sync up CMake files

Product:

WebKit

Reporter:

Rob Buis <rwlbuis>

Component:

WebKit BlackBerry

Assignee:

Rob Buis <rwlbuis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, rakuco, tonikitoo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	gyuyoung.kim: review+

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	Details Formatted Diff Diff
Patch (14.31 KB, patch) 2012-10-04 14:32 PDT, Rob Buis	gyuyoung.kim: review+	Details Formatted Diff Diff
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.