ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Information

1) Done
2) Done bumped the version and put the relevant stuff in the sections
3) I've commented out the 'dead code' where it uses the location code for TWC, let me know if you want me to remove

Also, I've spotted one todo on line 44 of the diff
+ var partData = dataObj["metric"] || dataObj["imperial"] || dataObj; // TODO: be more intelligent?

Do you think this is OK? or should it be more like?
if (there is metric) { use metric } else if (there is imperial) { use imperial } else { fallback }

Futhermore the lines below have things like calcFahrenheit() which convert from metric->imperial, so it assumes the data is metric, maybe we should remove the imperial option... so just

+ var partData = dataObj["metric"] || dataObj;

I think i've talked myself into changing it to that :')
-- 
https://code.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api/+merge/289584
Your team Ubuntu Weather Developers is subscribed to branch lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api.