Review Request 121534: [kalzium/libscience] Port to KF5

[Christoph Feck]

> On Dec. 16, 2014, 9:01 p.m., Etienne Rebetez wrote:
> > I dont know why KUnitConversion::UnitID was used. Probably just to show in the code, that this is an unit id?
> > Thanks for the porting initative:) I dont see the branch frameworks in git? Is there another repo?
> > Thanks.
> 
> Christoph Feck wrote:
>     No, because KUnitConversion framework was ported from int to enum
> 
> Christoph Feck wrote:
>     And there is no repo yet :)
> 
> Etienne Rebetez wrote:
>     Uups, sorry. I would be good if I could read...
>     Yes, enum is the nicer way to go. IMHO I the verbose way does not bother me.
> 
> Christoph Feck wrote:
>     Well, I did not like doing the changes for all Prefs::xxxUnit() accessors, see https://paste.kde.org/p04lci6lb
>     
>     I had hoped kconfig_compiler can create accessors which return an enum instead of an int, but it apparently cannot.
>     
>     My current idea is to keep the Kalzium code all using "int", but only convert this to the enum when actually doing the KUnitConversion calls, instead of porting all of Kalzium to using the enums.
>     
>     Which would you prefer?

Updated patch shows all changes that are required to port all of the code to the KUnitConversion::UnitId enum.

I now again think this is the best approach. Using "int" in the Kalzium APIs might result in a far smaller diff, but in the long term, the extra verbosity might increase readability.


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121534/#review72139
-----------------------------------------------------------


On Dec. 18, 2014, 11:02 p.m., Christoph Feck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121534/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2014, 11:02 p.m.)
> 
> 
> Review request for KDE Edu and Etienne Rebetez.
> 
> 
> Repository: kalzium
> 
> 
> Description
> -------
> 
> Not sure if anyone already looked at porting Kalzium. If not, I would like to create the frameworks branch, and push some changes I tried so far.
> 
> When I started porting the buildsystem I noticed many compile errors caused by API changes in KUnitConversion.
> 
> This review request only shows the changes inside libscience. The application does not compile yet.
> 
> I would like feedback about the (rather verbose) usage of KUnitConversion::UnitID type. Does it make sense to always "using namespace;" or maybe a typedef?
> 
> 
> Diffs
> -----
> 
>   libscience/element.cpp 762462f 
>   libscience/elementparser.h f3a8130 
>   libscience/elementparser.cpp 7c5ecaf 
>   libscience/isotopeparser.cpp 0e646c1 
>   libscience/spectrum.h 3d43d6f 
>   libscience/spectrum.cpp 9ce97a5 
>   src/calculator/concCalculator.cpp 785e77a 
>   src/calculator/gasCalculator.h 4cbcecf 
>   src/calculator/gasCalculator.cpp adef7f5 
>   src/calculator/nuclearCalculator.h f0230f8 
>   src/calculator/nuclearCalculator.cpp 488bd38 
>   src/elementdataviewer.cpp 49a2a18 
>   src/kalziumgradienttype.cpp 5130f85 
>   src/kalziumunitcombobox.h 717be9e 
>   src/kalziumunitcombobox.cpp 094ca45 
>   src/kalziumutils.h 0938b34 
>   src/kalziumutils.cpp bae9c54 
>   src/spectrumviewimpl.cpp f9c0488 
>   src/spectrumwidget.cpp 2264590 
>   src/unitsettingsdialog.h 04d5cf0 
>   src/unitsettingsdialog.cpp 4f50aca 
>   src/gradientwidget_impl.cpp 622717a 
>   src/kalziumdataobject.h 0d88856 
>   src/kalziumdataobject.cpp 3d171c6 
>   libscience/chemicaldataobject.h 55a40dc 
>   libscience/chemicaldataobject.cpp 8fac92c 
>   libscience/element.h 43979d9 
> 
> Diff: https://git.reviewboard.kde.org/r/121534/diff/
> 
> 
> Testing
> -------
> 
> libscience compiles without errors/warnings.
> 
> 
> Thanks,
> 
> Christoph Feck
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20141218/0d8e65a3/attachment.html>