Code review comment for lp:~neokore/ubuntu-weather-app/SquaredWeatherItems

Revision history for this message
Raúl Yeguas (neokore) wrote :

Thank you for your revision, I'll fix everything when I arrive home.

And sorry for breaking the autopilot tests, hehe.

Cheers!
El 15/08/2013 17:12, "Martin Borho" <email address hidden> escribió:

> Review: Needs Fixing
>
> Cool, very cool!
>
> Some things you should fix before we merge::
>
> - use FontUtils.sizeToPixels for the text sizes, see old version of
> CurrentWeather.qml. The text is too msall on the device.
>
> - Some test are broken now. Could you replace in
> tests/autopilot/ubuntu_weather_app/tests/test_settings.py in Line 40ff to
>
> if units == "imperial":
> self.assertThat(current_temps[0].text,
> Eventually(Equals(u'70')))
> self.assertThat(current_temps[1].text,
> Eventually(Equals(u'73')))
> else:
> self.assertThat(current_temps[0].text,
> Eventually(Equals(u'21')))
> self.assertThat(current_temps[1].text,
> Eventually(Equals(u'23')))
>
> - In WeatherTemperatureComponent could you please add
> objectName:"CurrentTempText" to the Text with id "value", so the tests
> above will work.
>
> - I've changed yesterday in
> tests/autopilot/ubuntu_weather_app/tests/test_locationmanager.py in Lines
> 121 and 137 "London, GB" and "Hamburg, DE" to "London, United Kingdom" and
> "Hamburg, Germany", since OWM has changed it. Could you switch it back?
> They've changed it back to the old values today and the tests are broken
> now.
>
> Cheers and Thanks
>
> Martin
>
>
> --
>
> https://code.launchpad.net/~neokore/ubuntu-weather-app/SquaredWeatherItems/+merge/180349
> You are the owner of lp:~neokore/ubuntu-weather-app/SquaredWeatherItems.
>

« Back to merge proposal