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