Re: KDEREVIEW: Mangonel
On Wed, Jan 23, 2013 at 7:13 PM, Martin Sandsmark martin.sandsm...@kde.org wrote: Distinguished Sirs and Madams, On Tue, Jan 08, 2013 at 09:08:11PM +0100, Martin Sandsmark wrote: Mangonel has just been moved to KDE Review. The KDE Review process is set to be two weeks, and if Mangonel calculator isn't completely wrong, that period is now up. Since all the issues raised are fixed, I assume I can now move forward with moving it to extragear/base? Mangonel has now been moved to Extragear Base. Thanks to everyone who have reviewed, and contributed comments and fixes. -- Martin Sandsmark Regards, Ben
Re: KDEREVIEW: Mangonel
Distinguished Sirs and Madams, On Tue, Jan 08, 2013 at 09:08:11PM +0100, Martin Sandsmark wrote: Mangonel has just been moved to KDE Review. The KDE Review process is set to be two weeks, and if Mangonel calculator isn't completely wrong, that period is now up. Since all the issues raised are fixed, I assume I can now move forward with moving it to extragear/base? Thanks to everyone who have reviewed, and contributed comments and fixes. -- Martin Sandsmark
Re: KDEREVIEW: Mangonel
On Wed, Jan 09, 2013 at 09:07:13PM +0200, Yuri Chornoivan wrote: If there is no Help button and no desktop file, there is not much sense in making docbooks. I agree. I propose just add an item to UserBase launchers list [1] and some tiny page based on README.md. I'll be sure to market it wherever I can when I finally release it. :-) -- Martin Sandsmark
Re: Re: KDEREVIEW: Mangonel
Hello List, On Tuesday 08 January 2013 23:12:01 Albert Astals Cid wrote: El Dimarts, 8 de gener de 2013, a les 21:08:11, Martin Sandsmark va escriure: Dear Sirs and Madams, Mangonel has just been moved to KDE Review. Mangonel is a simple and lightweight application launcher in the vein of Katapult from ye olde KDE 3 days, kind of reminiscent of the task oriented UI for krunner, but dropping the slow krunner architecture in favour of a simpler and more straightforward one that aims to give immediate feedback to the user. So it doesn't do everything krunner does, but should be useful for some people as either an alternative to or complement to krunner. It was originally developed by Bart Kroon, but he is currently too busy to work on it, so I got a go-ahead from him to clean it up for a proper release with licenses and stuff (which I think it needs because it's pretty useful and should be spread far and wide). Which is its intended destination extragear-something? Any reason not to use bugs.kde.org? It was not in KDE so it did not use bugs.kde.org. This would be appropriate when it is adopted obviously. The i18n is quite broken, a simple grep gives me ./Config.cpp:39:this-setWindowTitle(KApplication::instance()- applicationName() + QString( - Configuration)); ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show Mangonel:, this); ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel), this); ./Mangonel.cpp:92:QAction* actionConfig = new QAction(KIcon(configure), Configuration, this); ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( + application.type + ), this); Translations are indeed not a thing this program currently supports. There are not too many strings in there so this should not pose to much of a problem. Besides all those units in units.c seem untranslatable. Any reason for not using kunitconversion in there? I didn't know about this. This seems appropriate. Also the units.c copytight header looks a bit scary This was realy a quick hack add on that still needs a propper implementation. It doesn't even work properly as it is now. Cheers, Albert - Bart
Re: KDEREVIEW: Mangonel
On 2013年01月09日 08:09, Martin Sandsmark wrote: Any reason not to use bugs.kde.org? Fixed. Hi, I see you made the change : -aboutData-setBugAddress(QByteArray(bugs.mango...@tarmack.eu)); +aboutData-setBugAddress(QByteArray(https://bugs.kde.org/;)); Hmm, that is not going to work. Dr.Konqi expects sub...@bugs.kde.org as the bug address of applications using bugs.kde.org. If you plan to use bugs.kde.org as the tracker, then you don't need to call setBugAddress() at all. The default value just works. And don't forget to ask sysadmins to create a mangonel product on bugs.kde.org :) Regards Jekyll
Re: KDEREVIEW: Mangonel
On Wednesday 09 January 2013 08:56:25 Jekyll Wu wrote: If you plan to use bugs.kde.org as the tracker, then you don't need to call setBugAddress() at all. The default value just works. Fixed. And don't forget to ask sysadmins to create a mangonel product on bugs.kde.org :) Done. Thanks for the review! :D -- Martin Sandsmark KDE
Re: Re: KDEREVIEW: Mangonel
On Wednesday 09 January 2013 18:35:49 Martin Sandsmark wrote: On Tuesday 08 January 2013 19:30:45 Allen Winter wrote: No docbook manual I guess I'll to contact the doc team for this? Do you want apidox generated? if so you also need a Mainpage.dox No need for that (yet, at least, we might want to make it plugin-based in the future). Bart's email address is missing from the Copyright statements in many files. On purpose, I didn't want to spread his email address unencoded all over the internet. There is one mail already on the list, so it is already spread. signature.asc Description: This is a digitally signed message part.
Re: KDEREVIEW: Mangonel
El Dimecres, 9 de gener de 2013, a les 01:09:28, Martin Sandsmark va escriure: Hi, thanks for the review! On Tuesday 08 January 2013 23:12:01 Albert Astals Cid wrote: Which is its intended destination extragear-something? Yes, sorry, I forgot to mention, it is destined for extragear/base. Any reason not to use bugs.kde.org? Fixed. The i18n is quite broken, a simple grep gives me ./Config.cpp:39:this-setWindowTitle(KApplication::instance()- applicationName() + QString( - Configuration)); ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show Mangonel:, this); ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel), this); ./Mangonel.cpp:92:QAction* actionConfig = new QAction(KIcon(configure), Configuration, this); Fixed. ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( + application.type + ), this); application.type should already be translated, is it problematic that I wrap it in parentheses? You should probably make it look like i18nc(some context of what stuff is, (%1), application.type) Just in case someone needs to do something different with the parenthesis. Besides all those units in units.c seem untranslatable. Any reason for not using kunitconversion in there? Ported it to use KUnitConversion now. Also the units.c copytight header looks a bit scary Since they're not necessary anymore I just deleted units.c/units.h. (I hope I don't need to eradicate them from the git history?) No idea if we really need to be such purists Cheers, Albert And lastly, I also forgot to thank Bart for this great app. :-)
Re: KDEREVIEW: Mangonel
On Wednesday 09 January 2013 19:49:36 Albert Astals Cid wrote: You should probably make it look like i18nc(some context of what stuff is, (%1), application.type) Just in case someone needs to do something different with the parenthesis. Okay, fixed. Thanks :-) -- Martin Sandsmark KDE
Re: KDEREVIEW: Mangonel
On Wednesday 09 January 2013 19:45:27 Martin Gräßlin wrote: There is one mail already on the list, so it is already spread. Okay, so the proverbial cat's out of the bag I guess, so I just added his email to the license headers. -- Martin Sandsmark KDE
Re: KDEREVIEW: Mangonel
написане Wed, 09 Jan 2013 19:35:49 +0200, Martin Sandsmark martin.sandsm...@kde.org: On Tuesday 08 January 2013 19:30:45 Allen Winter wrote: No docbook manual I guess I'll to contact the doc team for this? If there is no Help button and no desktop file, there is not much sense in making docbooks. I propose just add an item to UserBase launchers list [1] and some tiny page based on README.md. Just my two cents. Best regards, Yuri [1] http://userbase.kde.org/Plasma_application_launchers
Re: KDEREVIEW: Mangonel
On Wednesday 09 January 2013 09:07:13 PM Yuri Chornoivan wrote: написане Wed, 09 Jan 2013 19:35:49 +0200, Martin Sandsmark martin.sandsm...@kde.org: On Tuesday 08 January 2013 19:30:45 Allen Winter wrote: No docbook manual I guess I'll to contact the doc team for this? If there is no Help button and no desktop file, there is not much sense in making docbooks. I propose just add an item to UserBase launchers list [1] and some tiny page based on README.md. Good idea. [1] http://userbase.kde.org/Plasma_application_launchers
KDEREVIEW: Mangonel
Dear Sirs and Madams, Mangonel has just been moved to KDE Review. Mangonel is a simple and lightweight application launcher in the vein of Katapult from ye olde KDE 3 days, kind of reminiscent of the task oriented UI for krunner, but dropping the slow krunner architecture in favour of a simpler and more straightforward one that aims to give immediate feedback to the user. So it doesn't do everything krunner does, but should be useful for some people as either an alternative to or complement to krunner. It was originally developed by Bart Kroon, but he is currently too busy to work on it, so I got a go-ahead from him to clean it up for a proper release with licenses and stuff (which I think it needs because it's pretty useful and should be spread far and wide). -- Martin Sandsmark KDE
Re: KDEREVIEW: Mangonel
El Dimarts, 8 de gener de 2013, a les 21:08:11, Martin Sandsmark va escriure: Dear Sirs and Madams, Mangonel has just been moved to KDE Review. Mangonel is a simple and lightweight application launcher in the vein of Katapult from ye olde KDE 3 days, kind of reminiscent of the task oriented UI for krunner, but dropping the slow krunner architecture in favour of a simpler and more straightforward one that aims to give immediate feedback to the user. So it doesn't do everything krunner does, but should be useful for some people as either an alternative to or complement to krunner. It was originally developed by Bart Kroon, but he is currently too busy to work on it, so I got a go-ahead from him to clean it up for a proper release with licenses and stuff (which I think it needs because it's pretty useful and should be spread far and wide). Which is its intended destination extragear-something? Any reason not to use bugs.kde.org? The i18n is quite broken, a simple grep gives me ./Config.cpp:39:this-setWindowTitle(KApplication::instance()- applicationName() + QString( - Configuration)); ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show Mangonel:, this); ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel), this); ./Mangonel.cpp:92:QAction* actionConfig = new QAction(KIcon(configure), Configuration, this); ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( + application.type + ), this); Besides all those units in units.c seem untranslatable. Any reason for not using kunitconversion in there? Also the units.c copytight header looks a bit scary Cheers, Albert
Re: KDEREVIEW: Mangonel
Hi, thanks for the review! On Tuesday 08 January 2013 23:12:01 Albert Astals Cid wrote: Which is its intended destination extragear-something? Yes, sorry, I forgot to mention, it is destined for extragear/base. Any reason not to use bugs.kde.org? Fixed. The i18n is quite broken, a simple grep gives me ./Config.cpp:39:this-setWindowTitle(KApplication::instance()- applicationName() + QString( - Configuration)); ./Config.cpp:48:QLabel* hotkeyLabel = new QLabel(Shortcut to show Mangonel:, this); ./Mangonel.cpp:74:m_actionShow = new KAction(QString(Show Mangonel), this); ./Mangonel.cpp:92:QAction* actionConfig = new QAction(KIcon(configure), Configuration, this); Fixed. ./Mangonel.cpp:480:m_descriptionLabel = new QGraphicsTextItem(( + application.type + ), this); application.type should already be translated, is it problematic that I wrap it in parentheses? Besides all those units in units.c seem untranslatable. Any reason for not using kunitconversion in there? Ported it to use KUnitConversion now. Also the units.c copytight header looks a bit scary Since they're not necessary anymore I just deleted units.c/units.h. (I hope I don't need to eradicate them from the git history?) And lastly, I also forgot to thank Bart for this great app. :-) -- Martin Sandsmark KDE
Re: KDEREVIEW: Mangonel
On Tuesday 08 January 2013 09:08:11 PM Martin Sandsmark wrote: Dear Sirs and Madams, Mangonel has just been moved to KDE Review. No docbook manual Do you want apidox generated? if so you also need a Mainpage.dox Bart's email address is missing from the Copyright statements in many files. Don't use QT4_AUTOMOC in the CMakeLists.txt There are some Krazy warnings you should also fix at some point. Hopefully the EBN will report those by tomorrow. I can email the Krazy report if you want it now. When running 'make install' I get this problem: -- Install configuration: debugfull -- Installing: /data/kde/bin/mangonel CMake Error at cmake_install.cmake:45 (FILE): file RPATH_CHANGE could not write new RPATH: /data/kde/lib:/data/Qt/Qt-4.8/lib:/data/kde/lib to the file: /data/kde/bin/mangonel Error opening file for update.
