D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Albert Astals Cid
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

D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Albert Astals Cid
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

D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-14 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-13 Thread Albert Astals Cid
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

D20284: Fix l/100 km to MPG conversion

2019-04-11 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-07 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-07 Thread Aleix Pol Gonzalez
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Nathaniel Graham
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Nathaniel Graham
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Albert Astals Cid
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Michal Malý
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

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Méven Car
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:

D20284: Fix l/100 km to MPG conversion

2019-04-06 Thread Michal Malý
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