[Differential] [Commented On] D360: KoFileDialog rather belongs to kowidgetutils than kowidgetsThis way apps can use the class without larger deps of kowidgets
kossebau added a comment. 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 :) INLINE COMMENTS 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. REVISION DETAIL https://phabricator.kde.org/D360 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: staniek, rempt, kossebau Cc: Calligra-Devel-list ___ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
[Differential] [Commented On] D360: KoFileDialog rather belongs to kowidgetutils than kowidgetsThis way apps can use the class without larger deps of kowidgets
staniek added a comment. In https://phabricator.kde.org/D360#7104, @kossebau wrote: > While moving filedialogtester, could you please move it into the subdir > tests/, so the normal dir only contains product code? 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 REVISION DETAIL https://phabricator.kde.org/D360 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: staniek, rempt, kossebau Cc: Calligra-Devel-list ___ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
[Differential] [Commented On] D360: KoFileDialog rather belongs to kowidgetutils than kowidgetsThis way apps can use the class without larger deps of kowidgets
staniek added a comment. In https://phabricator.kde.org/D360#7068, @kossebau wrote: > 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. REPOSITORY rCALLIGRA Calligra REVISION DETAIL https://phabricator.kde.org/D360 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: staniek, rempt, kossebau Cc: Calligra-Devel-list ___ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
