Review Request 120254: [Kalzium] Minor fixes, reorganize includes, some cleanups

Martin Walch walch.martin at web.de
Fri Jan 2 23:24:02 UTC 2015



> On Nov. 9, 2014, 10:08 nachm., Albert Astals Cid wrote:
> > So how are we going to proceed here? Is anyone willing to take reposnsability of the full commit? Should I ask for split reviews for the things i feel confident merging?
> 
> Albert Astals Cid wrote:
>     Nobody seems to want to take care of this big chunk so i'm going to discard the change, as said i feel confident with some others but not with all of them, if you want to propose them separately i'll review them as i can.

Sorry for not answering. I will pick this up again soon.


- Martin


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


On Jan. 2, 2015, 9:33 nachm., Martin Walch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120254/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2015, 9:33 nachm.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Repository: kalzium
> 
> 
> Description
> -------
> 
> * src/calculator/titrationCalculator.cpp:
> 
> replace
> > if (!texto) {
> >    QMessageBox::critical(this, "Error", "Unable to open " + file);
> > } 
> > if (texto) {
> 
> with
> > if (!texto) {
> >    QMessageBox::critical(this, "Error", "Unable to open " + file);
> > } else {
> 
> to emphasize that they are logically alternative
> 
> * src/calculator/titrationCalculator.cpp:
> Fix "warning: 'cn' may be used uninitialized in this function".
> As far as I see, this has no proper initialization at all and should
> horribly fail. However, in all my tests it behaved as if it was properly
> initialized with 0.0. Maybe there is some compiler (gcc) magic at work.
> Nevermind. Just fix it.
> 
> * libscience/spectrum.cpp:
> this line goes wrong
> > double newInt = p->intensity * 1000 / maxInt;
> because all three of p->intensity, 1000, and maxInt are Integers. Fix it
> with floating point number 1000.0. (The method is not used anywhere in
> Kalzium, but it is part of the libscience library)
> 
> * further cleanup of includes:
> 
> replace
> > #include <qclass.h>
> 
> with
> > #include <QtModule/QClass>
> 
> replace
> > #include <QtModule/QClass>
> 
> with
> > #include <QClass>
> 
> replace
> > #include <kclass.h>
> 
> with
> > #include <KClass>
> 
> replace
> > #include <QtModule>
> 
> with the classes of QtModule that are actually necessary.
> 
> use forward declarations and move includes into cpp files
> 
> remove duplicate or unused includes
> 
> (As a side note: single threaded build time reduced by roughly 5%)
> 
> * Drop C bindings in favor of explicit C++ bindings
> (e.g. include cctype, not ctype.h and use std namespace)
> 
> * Replace abs() and isfinite() with their corresponding Qt equivalents
> qAbs() and qIsFinite().
> 
> * Replace M_PI with const double PI = 3.141... as M_PI is bad for
> portability.
> 
> 
> Diffs
> -----
> 
>   compoundviewer/kalziumglpart.h 0881df1 
>   compoundviewer/kalziumglpart.cpp 001399d 
>   compoundviewer/openbabel2wrapper.cpp 85406e1 
>   libscience/chemicaldataobject.h 55a40dc 
>   libscience/chemicaldataobject.cpp 8fac92c 
>   libscience/element.h 43979d9 
>   libscience/element.cpp 762462f 
>   libscience/elementparser.h f3a8130 
>   libscience/elementparser.cpp 7c5ecaf 
>   libscience/isotope.cpp 91db538 
>   src/psetable/elementitem.cpp a2b238c 
>   src/psetable/numerationitem.h 5cf8afb 
>   src/psetable/numerationitem.cpp 574f74f 
>   src/psetable/periodictablescene.h c89044b 
>   src/psetable/periodictablescene.cpp 5ea99c0 
>   src/psetable/periodictablestates.h f8258c3 
>   src/psetable/periodictablestates.cpp 501b75e 
>   src/psetable/periodictableview.h 366480e 
>   src/psetable/periodictableview.cpp 9669e5b 
>   src/psetable/statemachine.h 0cad142 
>   src/psetable/statemachine.cpp 9ce8d58 
>   src/rsdialog.h 930276c 
>   src/rsdialog.cpp 64a6b60 
>   src/search.h 83bcf66 
>   src/search.cpp 7ff6464 
>   src/isotopetable/isotopeitem.h 5b7840b 
>   src/isotopetable/isotopeitem.cpp b98155d 
>   src/isotopetable/isotopescene.cpp 18f6a54 
>   src/isotopetable/isotopetabledialog.h 4f075c5 
>   src/isotopetable/isotopetabledialog.cpp 83fb000 
>   src/isotopetable/isotopeview.h 54d3d97 
>   src/isotopetable/isotopeview.cpp dd6bc8e 
>   src/kalzium.h 86e4a6d 
>   src/kalzium.cpp 2e85cf8 
>   src/kalziumdataobject.h 0d88856 
>   src/kalziumdataobject.cpp e962670 
>   src/kalziumelementproperty.h f8b3a67 
>   src/kalziumelementproperty.cpp c42ecd5 
>   src/kalziumgradienttype.cpp 5130f85 
>   src/kalziumnumerationtype.cpp 52275d0 
>   src/kalziumschemetype.h a6447d5 
>   src/kalziumschemetype.cpp d35a97c 
>   src/kalziumutils.h 0938b34 
>   src/kalziumutils.cpp bae9c54 
>   src/kdeeduglossary.h 3b47aee 
>   src/kdeeduglossary.cpp f561568 
>   src/legendwidget.h 9794f07 
>   src/legendwidget.cpp b0a7ba0 
>   src/main.cpp ff492f0 
>   src/molcalcwidget.h a0581a0 
>   src/molcalcwidget.cpp 784928a 
>   src/orbitswidget.cpp d94ccf1 
>   src/psetable/elementitem.h 7dd7e30 
>   src/searchwidget.cpp 17edc1b 
>   src/spectrumviewimpl.h 881609f 
>   src/spectrumviewimpl.cpp f9c0488 
>   src/spectrumwidget.h 4caecb4 
>   src/spectrumwidget.cpp 2264590 
>   src/tableinfowidget.h dcfeea7 
>   src/tableinfowidget.cpp 48eb865 
>   src/tablesdialog.h a04f4ad 
>   src/tablesdialog.cpp 6dd126e 
>   src/tools/moleculeview.h 73bf9b8 
>   src/tools/moleculeview.cpp 7a6c9f3 
>   src/tools/obconverter.cpp 04b557d 
>   src/unitsettingsdialog.h 04d5cf0 
>   src/unitsettingsdialog.cpp 4f50aca 
>   src/detailedgraphicaloverview.cpp 5fb716b 
>   src/detailinfodlg.h e81802b 
>   src/detailinfodlg.cpp 5932a7f 
>   src/elementdataviewer.h dba156e 
>   src/elementdataviewer.cpp 49a2a18 
>   src/eqchemview.h 59d3384 
>   src/eqchemview.cpp ebce0be 
>   src/exportdialog.h 7aa5543 
>   src/exportdialog.cpp a21dc26 
>   src/gradientwidget_impl.h 0521765 
>   src/gradientwidget_impl.cpp 622717a 
>   src/isotopetable/isotopeguideview.h 560f3ba 
>   src/isotopetable/isotopeguideview.cpp d96ce80 
>   plasmoid/applet/psePlasmoid/Periodictable.cpp 5b75778 
>   plasmoid/engine/kalzium_engine.h 3e46481 
>   plasmoid/engine/kalzium_engine.cpp 36fce6f 
>   src/calculator/calculator.h cda195d 
>   src/calculator/calculator.cpp efe57aa 
>   src/calculator/concCalculator.h 54c1fa1 
>   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/calculator/titrationCalculator.h 9880ebe 
>   src/calculator/titrationCalculator.cpp 6c67887 
>   src/detailedQmlView.h 6c30912 
>   src/detailedQmlView.cpp 50321f5 
>   src/detailedgraphicaloverview.h 49d9980 
>   plasmoid/applet/didyouknow/didyouknow.h 4f402fe 
>   plasmoid/applet/didyouknow/didyouknow.cpp 11a04a8 
>   plasmoid/applet/gasPlasmoid/gasCalculator.h f552742 
>   plasmoid/applet/gasPlasmoid/gasCalculator.cpp de78078 
>   plasmoid/applet/nuclearPlasmoid/kalziumdataobject.h 9189dea 
>   plasmoid/applet/nuclearPlasmoid/kalziumdataobject.cpp e802100 
>   plasmoid/applet/nuclearPlasmoid/nuclearCalculator.h d6ced65 
>   plasmoid/applet/nuclearPlasmoid/nuclearCalculator.cpp dfcbc6d 
>   plasmoid/applet/psePlasmoid/Molmasscalculator.h ccd80e5 
>   plasmoid/applet/psePlasmoid/Molmasscalculator.cpp 4b6acbe 
>   plasmoid/applet/psePlasmoid/Periodictable.h 5c130cc 
>   libscience/isotopeparser.h c59aff4 
>   libscience/isotopeparser.cpp 0e646c1 
>   libscience/moleculeparser.h 3a41b8b 
>   libscience/moleculeparser.cpp 35321cc 
>   libscience/parser.h 9c9e209 
>   libscience/parser.cpp e34f189 
>   libscience/psetables.cpp d7e12a0 
>   libscience/spectrum.h 3d43d6f 
>   libscience/spectrum.cpp 9ce97a5 
>   libscience/spectrumparser.h 1e60a1c 
>   libscience/spectrumparser.cpp c89c114 
>   libscience/tests/xmlreadingtest.cpp 90a0e25 
>   plasmoid/applet/bodr/kalzium_plasma.h 94083e2 
>   plasmoid/applet/bodr/kalzium_plasma.cpp 6f312b3 
>   plasmoid/applet/concentrationPlasmoid/concentrationCalculator.h 5c27286 
>   plasmoid/applet/concentrationPlasmoid/concentrationCalculator.cpp ac7f55b 
> 
> Diff: https://git.reviewboard.kde.org/r/120254/diff/
> 
> 
> Testing
> -------
> 
> * compiling
> * running program
> 
> 
> Thanks,
> 
> Martin Walch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20150102/b34cfd74/attachment-0001.html>


More information about the kde-edu mailing list