KTextEditor & Frameworks question

[David Faure]

On Tuesday 07 January 2014 19:57:56 Christoph Cullmann wrote:
> Hi,
> 
> I just tried to fix the naming issues.
> 
> Does that try here look better
> 
> http://quickgit.kde.org/?p=scratch%2Fcullmann%2Fktexteditor.git

I see a ${FooBar_HEADERS} in src/CMakeLists.txt which is used but not set,
should be removed.

src/include/CMakeList.txt is the one calling ecm_generate_headers, but in a 
strange way: it sets KTEXTEDITOR_PUBLIC_HEADERS which was already set,
so it overwrites it.
You can remove the whole list of lowercase headers, ecm_generate_headers 
generates it for you from the uppercase ones.
And then just install with the contents of that variable. *after* the call to 
ecm_generate_headers, not before ;)


> add_library (KF5TextEditor ${ktexteditor_LIB_SRCS} 
${KTEXTEDITOR_PUBLIC_HEADERS})

Why pass headers to add_library?

> 
> target_link_libraries(KF5TextEditor LINK_PUBLIC KF5::Parts
>                                   LINK_PRIVATE KF5::I18n
>     Qt5::Script
>     KF5::Archive
>     KF5::GuiAddons
>     KF5::I18n
>     KF5::IconThemes
>     KF5::ItemViews
>     KF5::KCMUtils
>     KF5::KIOFileWidgets
>     KF5::Notifications
>     KF5::Parts
>     KF5::PrintUtils
>     KF5::SonnetCore)

Are you sure that all of these should be private? The ones that provide 
classes that appear in the public API should be under LINK_PUBLIC.

> set( katepart_PART_UI
> )

unused, remove.

I tested the grafting, works fine.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5