Review request

Albert Astals Cid aacid at kde.org
Wed Dec 8 00:27:50 GMT 2010


A Dilluns, 6 de desembre de 2010, Thomas Baumgart va escriure:
> Hi folks,
> 
> I have just moved libalkimia as part of Alkimia (see
> http://techbase.kde.org/Projects/KdeFinance/Alkimia for details)  to
> kdereview for further processing by the community.
> 
> The plan is to move libalkimia to extragear/office/alkimia once approved
> and start using it in the various fincance apps under KDE. The start will
> be KMyMoney where we have a working patch to make that move but want to
> have the library in its current basic form in extragear first.
> 
> 
> Things that have been done so far:
> 
> - API documentation is provided
> - a set of unit testcases is provided
> - krazy checks have been run and don't report errors
> 
> 
> Note: there are no translatable strings included.
> 
> 
> What probably needs some input/modification from reviewers:
> 
> - cmake for more platforms/distros
> 
> 
> Thanks for all your work. Now awaiting your valuable input ...


  ~AlkValue() {}

Do not put the empty body there, maybe somewhen you'll decide that the 
destructors needs actual code, but programs compiled a header that includes 
the empty code in the header might decided to ignore the function call 
altogether and thus updating the library only might not make the program pick 
the "correct" destructor.


  AlkValue abs(void) const;

The void looks a bit C-ish to me but i guess it's ok.


Lots of inline functions. It's not a good idea to inline functions in a 
library unless you are really really really sure the code is not ever going to 
change.


Albert




More information about the kde-core-devel mailing list