Review Request 114182: Add Kf5 prefix for libs, eg. libKf5Foo

[Kevin Ottens]

> On Nov. 28, 2013, 3:43 p.m., Kevin Ottens wrote:
> > Looks fine to me. Makes me wonder if we want to drop the K for the target name "KF5::ConfigCore"? Just like Qt5Widgets becomes Qt5::Widgets...
> 
> Martin Klapetek wrote:
>     Can do. Imho it makes sense but I'll wait for some other opinions so I don't have to redo/undo all of it then. Will try to still do it by tonight.
> 
> Martin Klapetek wrote:
>     No comments on dropping the K? I'm about to do this...
> 
> Martin Gräßlin wrote:
>     +1
> 
> Martin Klapetek wrote:
>     While working on it, I realized that it may not be so good to drop the K from target name after all...unless we change the package names too, otherwise we'd be finding package KConfig and linking to KF5::Config, plus all the classes are named KConfig*...so the library in target_link_libs would be quite inconsistent with the rest.
>     
>     Please advise.
> 
> Alex Merry wrote:
>     If we want to follow Qt's example, we would rename the package KF5Config, and leave the classes as KConfig*.  This is a fairly substantial naming change to the packages, though, especially as the consistent thing would be to have KF5Solid and KF5ThreadWeaver, for example.
>     
>     Which I'm generally in favour of, actually.  I'm especially dubious about the names of the ItemModels and ItemViews packages, which are really quite generic.
> 
> Kevin Ottens wrote:
>     Yep, ItemModels and ItemViews would definitely benefit from being prefixed. XmlGui as well... Overall it looks more and more like welcome consistency to me.
> 
> Martin Klapetek wrote:
>     Ok, so to reiterate:
>     
>     lib names -> libKF5Archive, libKF5Config etc (capital F)
>     target names -> KF5::Archive, KF5::Config
>     package names -> KF5Archive, KF5Config
>     
>     correct?
> 
> Kevin Ottens wrote:
>     Works for me if no one has any problem with it.
> 
> Kevin Ottens wrote:
>     Oh, BTW, once we go for it, don't forget to cover plasma-framework too! Probably requires poking on #plasma or plasma-devel, it's kind of an "all or nothing" for the whole KDE Frameworks product.
> 
> David Faure wrote:
>     Looks nice for the examples you gave, but let's take KIO:
>     KIOCore will become lib=libKF5IOCore target=KF5::IOCore in package=KF5IO?  This dilutes the well-known name "KIO"...
>     Maybe in this particular case we want to repeat the K, i.e. libKF5KIOCore, KF5::KIOCore.
>     
>     libKF5Parts also won't look like KParts anymore. Well, I guess that one is a matter of getting used to :)

I see the point of those two where it's actually part of the brand name... That sais, one could argue it's the same for KConfig or KArchive really... So where is the line?


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114182/#review44703
-----------------------------------------------------------


On Nov. 28, 2013, 1:46 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114182/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2013, 1:46 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This is just one framework - KConfig - if this is the proper way to do this, I'll update this review with all frameworks updated to this.
> 
> 
> Diffs
> -----
> 
>   tier1/kconfig/autotests/CMakeLists.txt cc2626b 
>   tier1/kconfig/src/core/CMakeLists.txt c8a4842 
>   tier1/kconfig/src/gui/CMakeLists.txt b677d03 
>   tier1/kconfig/src/kconf_update/CMakeLists.txt 69668bc 
> 
> Diff: http://git.reviewboard.kde.org/r/114182/diff/
> 
> 
> Testing
> -------
> 
> Builds.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131129/f9084894/attachment.html>