Code review comment for lp:~vthompson/ubuntu-weather-app/fix-1286589

Revision history for this message
Martin Borho (martin-borho) wrote :

Hi Victor,

thanks, there's an error, you're right!

But the error lies in the function "calcMph(ms)", which does its job right for OWM, because their wind speed value is in meters per second. Where as TWC gives us km/h (not mph).

You can verify this, by calling the TWC one time with units=m and another time without. Also checking the TWC webpage gives a reference value to check against.

The fix you've proposed just makes the kmh value in TWC to the mph value. So the right fix, I guess, would be to change the calcMph() function in _buildDataPoint() *and* _buildDayFormat() of the TWC api to something like

function convertKmhToMph(kmh) {
   return kmh/1.609344
}

Cheers
Martin

review: Needs Fixing

« Back to merge proposal