| Summary: | Clean up Web Inspector target in Source/WebKit/blackberry/CMakeListsBlackBerry.txt | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||
| Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | jkjiang, jonathan.dong.webkit, rwlbuis, tonikitoo, webkit.review.bot, zimmermann | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 73144 | ||||||||
| Attachments: |
|
||||||||
|
Description
Daniel Bates
2011-11-18 18:34:02 PST
PR 123483 Created attachment 116651 [details]
Patch
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? Created attachment 116652 [details]
Patch
(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? (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! :) 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.
Comment on attachment 116652 [details] Patch Clearing flags on attachment: 116652 Committed r101191: <http://trac.webkit.org/changeset/101191> All reviewed patches have been landed. Closing bug. |