This revision was automatically updated to reflect the committed changes.
Closed by commit R292:bc7495095baa: Fix l/100 km to MPG conversion (authored by
madcatx, committed by aacid).
REPOSITORY
R292 KUnitConversion
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D20284?vs=56203=56215
aacid accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik, #frameworks, aacid
Cc: apol, aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
madcatx marked an inline comment as done.
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik, #frameworks, aacid
Cc: apol, aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
madcatx marked 2 inline comments as done.
madcatx added inline comments.
INLINE COMMENTS
> aacid wrote in fuel_efficiency.cpp:36
> I understand what you mean here, but i don't think that reciprocal is the
> word that describes this (are you a native speaker? if so maybe it's juts
> that my
madcatx updated this revision to Diff 56203.
madcatx added a comment.
More obvious explanation of the meaning of `m_isReciprocal`
REPOSITORY
R292 KUnitConversion
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D20284?vs=56201=56203
REVISION DETAIL
madcatx updated this revision to Diff 56201.
madcatx added a comment.
Added an inverse test with initial value in MPG
REPOSITORY
R292 KUnitConversion
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D20284?vs=55574=56201
REVISION DETAIL
https://phabricator.kde.org/D20284
aacid added inline comments.
INLINE COMMENTS
> convertertest.cpp:71
> +v = c.convert(v, QStringLiteral("l/100 km"));
> +QCOMPARE(v.number(), 8.0);
> }
Could you please also add a test where the initial value is in say MPG and then
we have the conversion to liters/100km, mpgi and l/100
madcatx marked an inline comment as done.
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik, #frameworks, aacid
Cc: apol, aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
madcatx added inline comments.
INLINE COMMENTS
> apol wrote in fuel_efficiency.cpp:36
> I'm not sure I understand what isReciprocal means here.
l/100 km (amount of fuel needed to cross a given distance) is reciprocal to MPG
(distance crossed with a given amount of fuel). KUnitConverter can
apol added inline comments.
INLINE COMMENTS
> fuel_efficiency.cpp:36
> +const KLocalizedString , const
> KLocalizedString ,
> +const bool isReciprocal = false)
> : UnitPrivate(categoryId, id, multiplier,
I'm not sure I understand what
ngraham added reviewers: Frameworks, aacid.
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik, #frameworks, aacid
Cc: aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
ngraham edited the summary of this revision.
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik
Cc: aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
madcatx updated this revision to Diff 55574.
madcatx added a comment.
1. Tweaked conversion factor to match those used by Google online converter
2. Added test to convert l/100 km to other representations and back
REPOSITORY
R292 KUnitConversion
CHANGES SINCE LAST UPDATE
aacid added a comment.
is having an autotest for this possible?
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To: madcatx, broulik
Cc: aacid, meven, kde-frameworks-devel, michaelh, ngraham, bruns
madcatx updated this revision to Diff 55512.
madcatx added a comment.
Tabs vs spaces
REPOSITORY
R292 KUnitConversion
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D20284?vs=55511=55512
REVISION DETAIL
https://phabricator.kde.org/D20284
AFFECTED FILES
src/fuel_efficiency.cpp
meven added inline comments.
INLINE COMMENTS
> fuel_efficiency.cpp:48
> +return unitMultiplier() / value;
> + return UnitPrivate::toDefault(value);
> }
Indentation seems off here
REPOSITORY
R292 KUnitConversion
REVISION DETAIL
https://phabricator.kde.org/D20284
To:
madcatx created this revision.
madcatx added a reviewer: broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
madcatx requested review of this revision.
REVISION SUMMARY
Previous code (probably) worked only for l/100 km -> MPG but not the other
way
17 matches
Mail list logo