Review Request 120254: [Kalzium] Minor fixes, reorganize includes, some cleanups
Albert Astals Cid
aacid at kde.org
Sun Sep 28 16:13:53 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120254/#review67567
-----------------------------------------------------------
I honestly still don't see the point of this commit, you're reordering some includes most of the times just for "perfection" of sorting, well, i'm sorry to tell you the next guy that comes and patches is going to break it, for those things you either have commit hooks or will break, you also remove some local includes like cctype in one file, well the C++ includes are different in lots of systems, do you know that it will not break for example std::isdigit in there?
For most of your changes i see no real gain and i can forsee some pain.
- Albert Astals Cid
On set. 20, 2014, 1:39 a.m., Martin Walch wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120254/
> -----------------------------------------------------------
>
> (Updated set. 20, 2014, 1:39 a.m.)
>
>
> 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/20140928/e564124a/attachment-0001.html>
More information about the kde-edu
mailing list