KDEREVIEW: Mangonel

[Albert Astals Cid]

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. :-)