KDEREVIEW: Mangonel

Martin Sandsmark martin.sandsmark at kde.org
Wed Jan 9 00:09:28 GMT 2013


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130109/f32a8314/attachment.htm>


More information about the kde-core-devel mailing list