shared lib: conversion-lib in Plasma

[Petri Damstén]

On Thursday 23 July 2009 23:52:17 Albert Astals Cid wrote:
> A Dijous, 23 de juliol de 2009, Carsten Niehaus va escriure:
> > Moin moin
> >
> > With the commits r1001601, r1001602 and r1001603 I moved the lib to
> > kde-review.
> >
> > I hope I did everything correctly... I never moved a lib...
> >
> > Let the review begin!
>
> * Might be worth Conversion namespace to be named KConversion,
> KUnitConversion ?
>
>  * Complex class has empty constructor and destructor defined in a header,
> move it to a cpp
>
>  * Do you really need Unit being a QObject?
>
>  * explicit keyword in Unit constructor it's worthless, only makes sense
> for constructors with 1 parameter
>
>  * Might make sense forward declaring KLocalizedString in unit.h instead of
> including it
>
>  * Might make sense forward declaring Unit and QVariant in value.h
>
>  * converter.h is installed and it includes lots of files that are not
> installed rendering the file unusable
>
>  * About translation, i see you have a Messages.sh that creates
> libconversion.pot but you don't load libconversion catalog in any place
> inside the library, if you have a single point of entry the user of the
> library is guaranteed to use once and only once you should add it there,
> otherwise you should either:
>  a) Retire the libconversion catalog when getting inside kdelibs and let
> the kdelibs4.po catalog swallow it all
>  b) Mark clearly in the documentation that one needs to insert the
> libconversion catalog when using the library.

I try to fix these after the weekend.

Petri

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090724/fd69597e/attachment.sig>