Bug 72784 – Clean up Web Inspector target in Source/WebKit/blackberry/CMakeListsBlackBerry.txt

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

Following up on Adam Barth's comment in bug #72768 (https://bugs.webkit.org/show_bug.cgi?id=72768#c8), we should clean up the Web Inspector target in Source/WebKit/blackberry/CMakeListsBlackBerry.txt so that it's more straight-forward to read and modify.

[Daniel Bates]

PR 123483

[Jonathan Dong]

Created attachment 116651 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 116651 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=116651&action=review

I'll r- this, as I have some open questions, and it needs a fixup in the ChangeLog.

> Source/WebKit/ChangeLog:9
> +		Abstracted the JavaScript file names from inspector/front-end/inspector.html
> +		to keep them in-sync with the changes of inspector.html.
> +
> +        Reviewed by NOBODY (OOPS!).

The order is wrong here, It needs to say Reviewed by... then the description should follow.
Also the indentation is off.

> Source/WebKit/blackberry/CMakeListsBlackBerry.txt:186
> +SET (JS_FILES ${JS_FILES} ${WEBKIT_DIR}/blackberry/WebCoreSupport/inspectorBB.js)

Hm, this seems useful for any CMake based build, no?
How does eg. the Efl port manage this?

[Jonathan Dong]

Created attachment 116652 [details]
Patch

[Nikolas Zimmermann]

(In reply to comment #4)
> Created an attachment (id=116652) [details]
> Patch
Can you answer my question regarding sharing this with other CMake builds systems?

[Jonathan Dong]

(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=116652) [details] [details]
> > Patch
> Can you answer my question regarding sharing this with other CMake builds systems?

Perhaps we are the only porting who implements the inspector feature for now and uses cmake as the build system.
Although EFL porting set ENABLE_INSPECTOR on by default, but it haven't implemented the inspector feature in WebKit/WebCoreSupport/InspectorClientEfl.cpp.
WinCE set ENABLE_INSPECTOR off by default and haven't implemented the inspector feature either.

So I'm not sure whether we need to share this with other cmake porting for now, is it appropriate to add an unnecessary target into the common cmake file that other build won't use? but if it's ok, we could move the custom target into WebKit/CMakeLists.txt. Would you give me some suggestion? thanks! :)

[Nikolas Zimmermann]

Comment on attachment 116652 [details]
Patch

This is completely sufficient, if no-one else uses inspector yet with CMake based builds. If this ever happens, it can be refactored. r=me.

[WebKit Review Bot]

Comment on attachment 116652 [details]
Patch

Clearing flags on attachment: 116652

Committed r101191: <http://trac.webkit.org/changeset/101191>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.