kdelibs splitting: April iteration

[Stephen Kelly]

Kevin Ottens wrote:

> Hello,
> 
> So March is gone and April already started. Because of some of March left
> overs the list for April is somewhat long.
> 
> # Frameworks list
> Here is the list of frameworks to be split with the person responsible for
> it[*] (who become de facto maintainers of said frameworks except if
> specified otherwise):

>  * kitemviews: Stephen Kelly;

Most of what I wrote that used to be in there has already been moved to 
tier1/itemmodels. Of what remains, the stuff that I wrote will have to move 
to a tier4 level, and the rest was written by ereslibre. Is he still around 
to maintain that stuff? I don't think I want to.

> If anyone sees a problem with those goals for April (apart from the points
> I address later on in this email), please speak now!
> 

I'm not sure you're progress tracking is using the right metrics and working 
towords the right goals. You're TODO lists are aiming at finished 
frameworks, but I rather think we should focus on the intermediate goals 
which enable those final goals. We should focus on the horizontal struts 
that tie otherwise unrelated code in kdelibs together (and 
interdependencies).

I'm thinking of intermediate goals like:

* Removing the use of classes that we no longer really need to have (eg 
KPushButton, more on that below), 
* Porting from K_GLOBAL_STATIC to Q_GLOBAL_STATIC (the Q_GLOBAL_STATIC which 
will be in Qt 5.1 will have the same API as the one which is in Qt4 - we can 
port the actual code now. If the Qt 5.1 API is a bit different, there will 
need to be a script to port Qt anyway, and we can use the same script to 
port in kdelibs if we port to the existing API now), 
* Porting from KGlobal::locale to KLocale::global, etc, 
* After the above, removing kglobal.h from files where it is no longer used 
(simplifying the task of creating a framework for the people who don't 
realize that that needs to be done, and reducing the build fixes that are 
required because they don't do a clean build after moving stuff around and 
changing include_directories)
* Actioning the tasks jlayt has listed regarding the future of KLocale 
(http://community.kde.org/KDE_Core/KLocale/Frameworks)
* Look into K-classes where Q classes exist, find out why the K-class 
exists, find out if the K-features are actually used, decide if it should 
remain, or be renamed to something more appropriate, document that in the 
wiki, and what changes in Qt are required, and/or make the changes. That is, 
investigate, decide, document and do. The result of such a TODO would either 
be documenting what should happen with a class, or doing it. The one line 
per class we have in the spreadsheet isn't complete enough, and doesn't 
focus on the K-classes where a Q-class exists, but lists all classes and is 
therefore not as focussed. There are probably less than 10 items that would 
need to be on this list. There was some effort on the KAction stuff, but I'm 
not sure the investigation took the right direction. I'd have to look again.


> # Why is the list large this month?
> As I pointed out, the list for this month is somewhat large, it contains
> 10 frameworks in total! I've been wondering if I should postpone some of
> the frameworks then and decided against for the following reasons:
>  * karchive, kaction, kguiaddons, kwidgets and kcolorwidgets got already
> started in the previous months and are close to be done anyway, so it's
> really just a matter of the people involved to not disappear and complete
> the work;

I looked through one of the branches a few weeks ago (the one that starts 
off moving colors stuff, then icons, kdialogs and kpushbutton etc), and have 
a rebased copy of it locally.

I think it takes the wrong approach to how we should process code that is 
not yet in a framework.

It's a lot of moving files around, and it seems that after some files were 
moved around, some other files were noticed as dependencies, so they were 
then moved too. There doesn't seem to have been any questioning of 'should 
this class even still exist anymore', with the exception of porting 
KColorDialog away from KHBox (which I have just cherry-picked into 
frameworks).

For example, KDialog was pulled in because the color dialogs use KDialog 
presumably. I don't know if KDialog should still exist, but in a later patch 
KPushButton was pulled in too, presumably because KDialog uses KPushButton. 
One of the problem is that the patches are upside-down (If KPushButton is a 
dependency, it should be moved before KDialog), but actually I don't think 
KPushButton needs to exist at all anymore.

If there is concensus on that, the code which is to be moved should instead 
first be ported away from KPushButton before being moved to a framework.

The reasons I don't think KPushButton needs to exist are: 
* On top of QPushButton ctors, it adds a KGuiItem ctor. I don't think 
KGuiItem needs to exist tbh, because I don't think it's good API. It's main 
feature is large ctors, which if you listen to any troll talking about Qt4 
API compared to Qt3, is an antipattern. The comments of the KGuiItem class 
even warn about this (It's not doxygen docs in its header). It also has 
setFoo API, but if you're going to call guiItem.setFoo(...); 
button.setGuiItem(guiItem), why not just call button.setFoo() instead? 
** I don't really have a problem with KGuiItem continuing to exist. Uses of 
it could be 'exploded' to individual calls to QPushButton (if KDialog is 
ported away from KPushButton), such as in KDialog::setButtonGuiItem. A 
KGuiItem equivalent might be able to make it into Qt either, but would have 
to be written, and would have to be implicitly shared (and used in Qt 
widgets). This again, is something that just needs to be investigated (and 
brought up on the Qt list), decided, documented and done.
* KPushButton also adds some APIs for dragging. The only user of this in 
kdelibs is kio/kfile/kurlrequester.cpp. Are there more users outside of 
kdelibs? Does it really make sense to have it? Why doesn't Qt have similar 
API?
* KPushButton also has some concept of a 'delayed' menu. I don't know what 
that is - again something to be investigated. The docs for it complain about 
menu() not being virtual [in Qt]. That is something which I think can still 
be changed in Qt, if there is a justification for it.
* The last remaining part of KPushButton is the KAuth action stuff. If 
that's the only reason for it remain, then it should be renamed 
KAuthPushButton, so that the purpose is clear, and so that people know that 
they don't have to use KPushButton instead of QPushButton.

Apart from the above, when people create a KPushButton, they just use 
QPushButton API with it. Mostly the K-features aren't used. After 
investigating all that stuff fully, what should be done with it should 
become more obvious. The case is similar for other K-classes, so the above 
is what the TODO lists should look like. 

Additionally, one or two of the patches change 
KGlobalSettings::dndEventDelay() to QApplication::startDragDistance(), but 
only in the files that are being moved around to the staging directory. This 
is again, one of the horizontal struts that I mentioned above that get in 
the way when creating a framework.

Changing KGlobalSettings::dndEventDelay() to 
QApplication::startDragDistance() should simply be one of our todo items, 
and we should simply change of them at once in all of kdelibs in the 
frameworks branch. I do find it an odd change though, because dndEventDelay 
sounds like it's related to time (I haven't looked at its docs), and 
startDragDistance seems to relate to distance. At least, porting probably 
can't be done blindly.

We shouldn't put items of 'splitting frameworks out' onto our TODO list, but 
rather things I have listed in this email IMO. The problem with splitting 
frameworks out is that it's harder than the things I have listed here. 
People don't know how to do it, as evidenced by the commits in the branches, 
which is of course understandable. It requires knowledge of CMake, what 
include directories are, what exports and export macros are, what linking 
is, etc. I think splitting (a vertical orientated task) is more difficult 
than decoupling (a horizontal task), and so we shouldn't ask newbies to do 
splitting. It's more frustrating and mysterious than it needs to be I think.

> 
> # RFC: rebase frameworks-kactions and frameworks-kcolors branches
> The work on kaction, kguiaddons, kwidgets and kcolorwidgets is for now
> happening in two separate branches. I think that was a good move at the
> time to get Giorgos and Antonis up to speed. But now that they are in a
> somewhat good shape (only kdeguiaddons seem to need some renaming at that
> point), it's probably time to rebase them on top of the frameworks branch
> and kill them to give more exposure to that work. Any opinion?

Maybe. I'd like to finalize some of the decisions around KPushButton, 
KGuiItem, KDialog etc first and get those changes into the frameworks 
branch, then it will be easier to rebase those branches, and it will be 
easier to define why the split-out framework exists. 

I think for any class that we're putting into a framework, we should be able 
to answer the question 'Why would a developer using Qt use this class?'. For 
KPushButton I think the closest answer might be 'If he's using KAuth', but 
it needs to be more fully investigated. 

For the icons stuff, I think there should be some renaming. The engine class 
there is an implementation of QIconEngine which loads icons from an Xdg 
theme, right? So it should be KXdgIconEngine. The answer to the question of 
why use it would then be 'Use it if you want to be able to load themes which 
follow the xdg icon spec', if the engine is generic enough for that...

I can also see a reason for the color widgets to exist in a framework.

I don't like that kwindowsystem was pulled in as a dependency into that 
framework though. It's not clear from the history why that was needed. This 
is also something that should be investigated, rather than simply pulled in 
as a dependency. Frameworks is largely about breaking up interdependencies 
where they are not really needed, so simply pulling all additional 
dependencies when doing splits isn't really productive towards that goal.

On the subject of kwindowsystem, I saw it was moved out of staging. Given 
that it doesn't work with Qt 5, I think it should still be in staging. 
Moving to a tier should indicate that a framework is 'done done' at least in 
the sense of building in the environment we want it to work in (that is, 
against Qt 5). I think it should be moved back.

I also think that there should be no use of Q_WS_* in tier frameworks. 
Currently KAuth has a few uses of them (in its CMake stuff), but they can be 
removed quite easily. I can maybe do that later. I think being clean of that 
stuff should be part of the checklist before considering a framework done 
though.

> 
> 
> Thanks for listening, feedback and comments are welcome.

I hope this is useful feedback for you. 

If you agree with me, we can work on a new TODO list, and some goals which I 
think are more realistic to achieve also for newbies, and which help the 
effort of making the useful parts of kdelibs available to people who use Qt.

Thanks,

Steve.