<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127193/">https://git.reviewboard.kde.org/r/127193/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On März 1st, 2016, 12:56 nachm. UTC, <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am now satisfied with the current state and would like to propose it for merge into master.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 :)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- Kai Uwe</p>
<br />
<p>On März 1st, 2016, 12:43 nachm. UTC, Friedrich W. H. Kossebau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma and Marco Martin.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated März 1, 2016, 12:43 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeplasma-addons
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Layout:
<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">configGeneral.qml</code> needs larger minimal sizes of the input fields. How can this be done? I am lost with the QQ2 layout approach for now.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Q: Is this ported correctly (a hack I found elsewhere), or is there a better way meanwhile?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> q->setBusy(false);
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">to</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> QObject *graphicObject = q->property("_plasma_graphicObject").value<QObject *>();
if (graphicObject) {
graphicObject->setProperty("busy", false);
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Q: <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Plasma::Applet::showMessage()</code> can be ported to what?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Q: <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">DataEngine::query()</code> should be ported to? Similar code is unported also in the weather/ions/ion_noaa dataengine in plasma-workspace.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> 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);
</pre></div>
</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(5568251)</span></li>
<li>applets/CMakeLists.txt <span style="color: grey">(bd3ea48)</span></li>
<li>applets/weather/CMakeLists.txt <span style="color: grey">(354b6ee)</span></li>
<li>applets/weather/package/contents/config/config.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/package/contents/ui/DetailsView.qml <span style="color: grey">(bbcdd15)</span></li>
<li>applets/weather/package/contents/ui/FiveDaysView.qml <span style="color: grey">(a39f334)</span></li>
<li>applets/weather/package/contents/ui/Notice.qml <span style="color: grey">(d38c108)</span></li>
<li>applets/weather/package/contents/ui/NoticesView.qml <span style="color: grey">(16923c4)</span></li>
<li>applets/weather/package/contents/ui/TopPanel.qml <span style="color: grey">(af57f45)</span></li>
<li>applets/weather/package/contents/ui/Utils.js <span style="color: grey">(f2d5b27)</span></li>
<li>applets/weather/package/contents/ui/WeatherListView.qml <span style="color: grey">(b29099f)</span></li>
<li>applets/weather/package/contents/ui/configGeneral.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/package/contents/ui/main.qml <span style="color: grey">(c501ab3)</span></li>
<li>applets/weather/package/metadata.desktop <span style="color: grey">(79646c2)</span></li>
<li>applets/weather/plasma-applet-weather.desktop <span style="color: grey">(aca6da2)</span></li>
<li>applets/weather/plugin/abstractunitlistmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/abstractunitlistmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/locationlistmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/locationlistmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/plugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/plugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/plugin/qmldir <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/weather/weatherapplet.h <span style="color: grey">(c4b376b)</span></li>
<li>applets/weather/weatherapplet.cpp <span style="color: grey">(60f882a)</span></li>
<li>libs/CMakeLists.txt <span style="color: grey">(bd71119)</span></li>
<li>libs/plasmaweather/CMakeLists.txt <span style="color: grey">(a9faa7b)</span></li>
<li>libs/plasmaweather/plasmaweather.knsrc <span style="color: grey">(8525f20)</span></li>
<li>libs/plasmaweather/plasmaweather_export.h <span style="color: grey">(691db23)</span></li>
<li>libs/plasmaweather/weatherconfig.h <span style="color: grey">(9b3c2d7)</span></li>
<li>libs/plasmaweather/weatherconfig.cpp <span style="color: grey">(1ec0b42)</span></li>
<li>libs/plasmaweather/weatherconfig.ui <span style="color: grey">(f285fff)</span></li>
<li>libs/plasmaweather/weatheri18ncatalog.h <span style="color: grey">(0122378)</span></li>
<li>libs/plasmaweather/weatheri18ncatalog.cpp <span style="color: grey">(1868352)</span></li>
<li>libs/plasmaweather/weatherlocation.h <span style="color: grey">(004d788)</span></li>
<li>libs/plasmaweather/weatherlocation.cpp <span style="color: grey">(1d275ea)</span></li>
<li>libs/plasmaweather/weatherpopupapplet.h <span style="color: grey">(ce95a5a)</span></li>
<li>libs/plasmaweather/weatherpopupapplet.cpp <span style="color: grey">(4533619)</span></li>
<li>libs/plasmaweather/weathervalidator.h <span style="color: grey">(eb33558)</span></li>
<li>libs/plasmaweather/weathervalidator.cpp <span style="color: grey">(4d016e2)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127193/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>