Code review comment for lp:~ubuntu-weather-dev/ubuntu-weather-app/new-weather-api

Revision history for this message
Victor Thompson (vthompson) wrote :

Thanks for finishing this off, Andrew!!

I ran AP and all tests pass. Code changes look fine so far, but one thing we might want to take the opportunity to do now (otherwise we may never do so) is to audit our code coverage in WeatherApi.js and remove any dead code so we don't do needless maintenance on unused code in the future. QtCreator doesn't seem to have a coverage tool, but there seem to be numerous third party tools and qml plugins that might be helpful. Maybe just a JS only coverage tool would be sufficient.

I've been testing this change out and have not hit any regressions thus far. Unless we want to look at removing any dead code, I think this is ready for QA.

TODO:
1. Update changelog
2. Bump version number?
3. Potentially look at removing any dead code

review: Needs Fixing

« Back to merge proposal