shared lib: conversion-lib in Plasma
Albert Astals Cid
aacid at kde.org
Thu Jul 23 21:52:17 BST 2009
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
> 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
* 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
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.
More information about the kde-core-devel