Review Request 127193: Revival of Weather applet (for Plasma 5.6)

Sebastian Kügler sebas at kde.org
Tue Mar 1 14:09:44 UTC 2016 


> On March 1, 2016, 12:56 p.m., Friedrich W. H. Kossebau wrote:
> > I am now satisfied with the current state and would like to propose it for merge into master.
> > 
> > From what I saw there is one issue still left: sometimes after changing the config there is no immediate data update coming to the applet. As the applet resets its UI to empty by default (there is no different handling of weather station change and unit changes), this then results in an empty applet, until the next data is delivered (also results in the busy indicator on the applet, as it waits for the data and times out only after 2min).
> > But this is old logic of the code, so not a regression, at least in the applet (I have yet to see why the data is not pushed to the applet).
> > Thus I would argue this should not be a showstopper for the merge.
> > Will look into fixing this anyway, i.e. split adapting to config changes. But for 5.6 I would like to concentrate on getting more weather ion plugins ported still today.
> > 
> > So please give this applet another test and consider getting it in for 5.6, I surely am not the only one missing it a lot :)
> 
> Kai Uwe Broulik wrote:
>     I think fixing the two minutes busy indicator is important as it causes significant cpu load for many users. I'm fine with 5.6 for it given it's not a core applet and we still have the beta to collect some more feedback. If all breaks we can still remove it from final.

I agree that we should ship it rather now, than wait for 5.7. Nothing to be gained from waiting another cycle.


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127193/#review92991
-----------------------------------------------------------


On March 1, 2016, 12:43 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127193/
> -----------------------------------------------------------
> 
> (Updated March 1, 2016, 12:43 p.m.)
> 
> 
> Review request for Plasma and Marco Martin.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> -------
> 
> So the weather applet in branch kossebau/weatherapplet works okayish now here, both on panel and on containment:
> 	https://share.kde.org/index.php/s/MxwDYtmRw9TtQ8X
> 
> This revival is done as minimal porting, after more considerations I for now kept libplasmaweather as it is where possible (also because I use classes from the lib in the QML plugin and in the applet, so some shared unit is needed).
> I propose to do further modernizing & refactoring of the applet code only once it works again completely and can be part of 5.6 release.
> 
> Please help with a quick review. And also with a few issues that still need to be solved for a perfect solution. Where I need your, the experts', help, please read on.
> 
> Layout:
> `configGeneral.qml` needs larger minimal sizes of the input fields. How can this be done? I am lost with the QQ2 layout approach for now.
> 
> Size of applet in fullRepresentation mode when expanded from panel:
> while the expanded applet had some kind of sane size all the time I was playing (cmp. the screenshot), at one point the size of the expanded representation only turned into some unusable 16x16 or similar size. For existing applets and new ones.
> 
> 
> Q: Is this ported correctly (a hack I found elsewhere), or is there a better way meanwhile?
> ```
>     q->setBusy(false);
> ```
> to
> ```
>     QObject *graphicObject = q->property("_plasma_graphicObject").value<QObject *>();
>         if (graphicObject) {
>             graphicObject->setProperty("busy", false);
>         }
> ```
> 
> Q: `Plasma::Applet::showMessage()` can be ported to what?
> 
> Q: `DataEngine::query()` should be ported to? Similar code is unported also in the weather/ions/ion_noaa dataengine in plasma-workspace.
> 
> ```
>         Plasma::DataEngine::Data data = timeEngine->query(
>                  QString(QLatin1String( "Local|Solar|Latitude=%1|Longitude=%2" )).arg(latitude).arg(longitude));
>         bool day = data[QLatin1String( "Corrected Elevation" )].toDouble() > 0.0);
> ```
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 5568251 
>   applets/CMakeLists.txt bd3ea48 
>   applets/weather/CMakeLists.txt 354b6ee 
>   applets/weather/package/contents/config/config.qml PRE-CREATION 
>   applets/weather/package/contents/ui/DetailsView.qml bbcdd15 
>   applets/weather/package/contents/ui/FiveDaysView.qml a39f334 
>   applets/weather/package/contents/ui/Notice.qml d38c108 
>   applets/weather/package/contents/ui/NoticesView.qml 16923c4 
>   applets/weather/package/contents/ui/TopPanel.qml af57f45 
>   applets/weather/package/contents/ui/Utils.js f2d5b27 
>   applets/weather/package/contents/ui/WeatherListView.qml b29099f 
>   applets/weather/package/contents/ui/configGeneral.qml PRE-CREATION 
>   applets/weather/package/contents/ui/main.qml c501ab3 
>   applets/weather/package/metadata.desktop 79646c2 
>   applets/weather/plasma-applet-weather.desktop aca6da2 
>   applets/weather/plugin/abstractunitlistmodel.h PRE-CREATION 
>   applets/weather/plugin/abstractunitlistmodel.cpp PRE-CREATION 
>   applets/weather/plugin/locationlistmodel.h PRE-CREATION 
>   applets/weather/plugin/locationlistmodel.cpp PRE-CREATION 
>   applets/weather/plugin/plugin.h PRE-CREATION 
>   applets/weather/plugin/plugin.cpp PRE-CREATION 
>   applets/weather/plugin/qmldir PRE-CREATION 
>   applets/weather/weatherapplet.h c4b376b 
>   applets/weather/weatherapplet.cpp 60f882a 
>   libs/CMakeLists.txt bd71119 
>   libs/plasmaweather/CMakeLists.txt a9faa7b 
>   libs/plasmaweather/plasmaweather.knsrc 8525f20 
>   libs/plasmaweather/plasmaweather_export.h 691db23 
>   libs/plasmaweather/weatherconfig.h 9b3c2d7 
>   libs/plasmaweather/weatherconfig.cpp 1ec0b42 
>   libs/plasmaweather/weatherconfig.ui f285fff 
>   libs/plasmaweather/weatheri18ncatalog.h 0122378 
>   libs/plasmaweather/weatheri18ncatalog.cpp 1868352 
>   libs/plasmaweather/weatherlocation.h 004d788 
>   libs/plasmaweather/weatherlocation.cpp 1d275ea 
>   libs/plasmaweather/weatherpopupapplet.h ce95a5a 
>   libs/plasmaweather/weatherpopupapplet.cpp 4533619 
>   libs/plasmaweather/weathervalidator.h eb33558 
>   libs/plasmaweather/weathervalidator.cpp 4d016e2 
> 
> Diff: https://git.reviewboard.kde.org/r/127193/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160301/3199f75b/attachment.html>

More information about the Plasma-devel mailing list