This way apps can use the class without larger deps of kowidgets
KoFileDialog belongs to "Tier 0" Also move filedialogtester to widgetutils: * remove all i18n - not needed for the test * rename to kofiledialogtester * move to tests/ subdir * remove main.cpp which is too general file, place main() in KoFileDialogTester.cpp * by the way remove link to ki18n in KoPropertiesTest
build
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Issue in any case: this patch misses to also move the test app, filedialogtester.
Then I wonder if we could first come up with a definition what the purpose of kowidgets and kowidgetutils is. And if perhaps we should split them up some more or reshuffle together with other libs.
Especially would I like to see a split between QWidget things and non-gui classes, so QtQuick solutions do not get trojan libs which come with QWidget stuff.
Other than that I would agree that KoFileDialog might be rather belong into kowidgetutils, due to not having a dep on other stuff in the Calligra libs, if that is the current difference between kowidgets and kowidgetutils.
Yeah, not using kowidgets in Kexi is my top need.
I try to avoid forking code even temporarily. It's enough that I forked bits of kdelibs4support.
I see kowidgetutils like kexiutils: Tier 0. For now I don't care about dependency on QWidgets but one day an utils lib without QWidgets (Tier -1?) would be handy too.
So I see you want me to update for the filedialogtester...
PS: Quite similarly I'll try to make KIO optional in Kexi so one can add KIO support like a plugin. (that's a good dream) For now some places use local QFiles, some use urls.
Agree on removal of i18n, not really needed IMHO, let's have translators only spend time on enduser facing strings :)
While moving filedialogtester, could you please move it into the subdir tests/, so the normal dir only contains product code?
As you just moved the KoFileDialog class files (and updated used export macro), I have not really looked into the code, only the CMakeLists.txt changes. Also not tested, assuming things work as before as kowidgetutils is a public dep of kowidgets :)
With filedialogtester moved down to subdir tests/, seems fine to me and good to ship.
OK, I wanted to propose that.
As you just moved the KoFileDialog class files (and updated used export macro), I have not really looked into the code, only the CMakeLists.txt changes. Also not tested, assuming things work as before as kowidgetutils is a public dep of kowidgets :)
With filedialogtester moved down to subdir tests/, seems fine to me and good to ship.
OK
Not sure I would have moved the main.cpp into the other file, but there is no policy in Calligra about such utils apps, so if you prefer it like that, keep it as you did now. (I prefer having entry points in a separate file, even do main.cpp files for plugins, but I know that this is my personal style only. And as long the util app is sharing the same folder with other stuff, the main.cpp could be conflicting, so...).
So with the request to have to the fix to KoPropertiesTest in a separate commit, this patch here seems fine to me to go in. Still untested and only partially code-reviewed, as before, with same reasoning :)
| libs/widgetutils/tests/CMakeLists.txt | ||
|---|---|---|
| 8 | This is fixed in a separate commit, right? If not, please make this a separate commit, for improved clear scopes of the atomic changes by commits. | |
Using the KF5 naming, this is a test, and our QTests are called autotests. There, tests/ contain (interactive) apps. Example:
ktextwidgets/tests/ktextedittest.cpp
We're not yet following this scheme in calligra (no idea if we want this extra rename) but I use it in KDb, etc. I followed KF5 already. The third type of dir is example/ - a real-world app.
(that's for the record, sorry for repeating obvious things)
Thx!