[Webkit-unassigned] [Bug 78570] [DRT] Remove all PlainTextController usages in existing tests by adding internal API.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 29 00:08:54 PST 2012


https://bugs.webkit.org/show_bug.cgi?id=78570





--- Comment #25 from Patrick R. Gansterer <paroga at paroga.com>  2012-02-29 00:08:52 PST ---
(From update of attachment 129399)
View in context: https://bugs.webkit.org/attachment.cgi?id=129399&action=review

i still like to see this patch split up

I updated the patch at bug 72816 with my current diff

> Source/WebCore/CMakeLists.txt:96
> +SET(WebCoreTestSupport_INCLUDE_DIRECTORIES
> +    "${WebCore_INCLUDE_DIRECTORIES}"
> +    "${WEBCORE_DIR}/testing"
> +)
> +

I'd prefer to set all WebCoreTestSupport_* variables after the WebCore_* variables (like we do with the features). IMHO this will make it easier to find which files are related to WebCoreTestSupport without searching across the big file.

> Source/WebCore/CMakeLists.txt:2522
> +IF (NOT WebCoreTestSupport_LIBRARY_NAME)
> +    SET(WebCoreTestSupport_LIBRARY_NAME webcoretestsupport)
> +ENDIF ()

Can you please add at SET(WebCoreTestSupport_LIBRARY_NAME WebCoreTestSupport) in the root CMakeLists.txt (like the other *_LIBRARY_NAME)

> Source/WebCore/CMakeLists.txt:2524
> +WEBKIT_WRAP_SOURCELIST(${WebCore_IDL_FILES} ${WebCore_SOURCES} ${WebCoreTestSupport_IDL_FILES} ${WebCoreTestSupport_SOURCES})

I'd prefere two call: 1 for WebCore and 1 for WebCoreTestSupport, but its ok for me too, if you prefer it that way :-)

> Source/WebCore/CMakeLists.txt:2529
> +ADD_LIBRARY(${WebCoreTestSupport_LIBRARY_NAME} ${WebCore_LIBRARY_TYPE} ${WebCoreTestSupport_SOURCES})

please create a new LIBRAR_TYPE. WCTS will be usually a static lib only used in DRT

> Source/WebCore/CMakeLists.txt:2532
> +ADD_DEPENDENCIES(${WebCoreTestSupport_LIBRARY_NAME} ${JavaScriptCore_LIBRARY_NAME} ${WebCore_LIBRARY_NAME})

is JSC needed when WebCore is there?

> Source/WebCore/CMakeLists.txt:2535
> +TARGET_LINK_LIBRARIES(${WebCoreTestSupport_LIBRARY_NAME} ${WebCore_LIBRARIES})

Do we need all WebCore libraries for WCTS?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list