-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2026/
-----------------------------------------------------------

(Updated 2009-11-01 18:31:18.178318)


Review request for kdelibs, Plasma and Shawn Starr.


Changes
-------

Incorporated a couple of suggestions from the review comments, such as:
- Renamed getForecast into fetchForecast
- Use QString::fromLatin1 where appropriate
- Removed QObject inheritance for WetterComIon::Private dptr class
- Cleaned up the code

I also had a look into moving the private methods from WetterComIon into 
WetterComIon::Private, but after having read some more documents about the 
intention of d-pointers as lightweight data containers to ensure binary API 
compatibility across versions I do not think that this would make too much 
sense. @Armin: Did I misinterpret your comment?

I also got a reply from wetter.com that they are looking into the umlaut search 
issue.


Summary
-------

This change brings support for wetter.com (a weather forecast web site 
especially popular in Germany) as a weather data source to KDE.
The Ion makes use of wetter.com's officially-supported REST API. The API 
supports forecasts for up to three days, so the Ion does not supply any 
information about the current weather conditions. The forecast for the current 
day is split between night and day whereas the following two days are provided 
as an aggregated value for the whole day, respectively. 

Any feedback is much appreciated!


This addresses bug 204219.
    https://bugs.kde.org/show_bug.cgi?id=204219


Diffs (updated)
-----

  /trunk/kdereview/plasma/dataengines/CMakeLists.txt 1043003 
  /trunk/kdereview/plasma/dataengines/weather/CMakeLists.txt PRE-CREATION 
  /trunk/kdereview/plasma/dataengines/weather/ions/CMakeLists.txt PRE-CREATION 
  /trunk/kdereview/plasma/dataengines/weather/ions/ion-wettercom.desktop 
PRE-CREATION 
  /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.h PRE-CREATION 
  /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.cpp 
PRE-CREATION 

Diff: http://reviewboard.kde.org/r/2026/diff


Testing
-------

Tested the Ion under a current KDE installation from trunk, fixed all Krazy 
warnings before the move to kdereview. Profiled it for memory leaks using 
Valgrind, which did not bring up any leaks in the control of the Ion. Those two 
still look somewhat suspicious, so I'd like to get your opinion about them:

42 bytes in 1 blocks are definitely lost in loss record 372 of 1,156
   at 0x4C25153: malloc (vg_replace_malloc.c:195)
   by 0x60EDCB4: QString::QString(int, Qt::Initialization) (qstring.cpp:1027)
   by 0x61BD5EE: QUtf8::convertToUnicode(char const*, int, 
QTextCodec::ConverterState*) (qutfcodec.cpp:169)
   by 0x60EFDFA: QString::fromUtf8(char const*, int) (qstring.cpp:3850)
   by 0x614C052: fromPercentEncodingMutable(QByteArray*) (qurl.cpp:223)
   by 0x614EFB3: QUrlPrivate::parse(QUrlPrivate::ParseOptions) const 
(qurl.cpp:3781)
   by 0x61534FE: QUrl::isValid() const (qurl.cpp:4124)
   by 0x5A23B5A: KUrl::_setEncodedUrl(QByteArray const&) (kurl.cpp:1538)
   by 0x5A23F19: KUrl::KUrl(QString const&) (kurl.cpp:411)
   by 0x1FAE37D6: WetterComIon::getForecast(QString const&) 
(ion_wettercom.cpp:504)
   by 0x1FAE5BAA: WetterComIon::updateIonSource(QString const&) 
(ion_wettercom.cpp:320)
   by 0x1F267CBE: IonInterface::sourceRequestEvent(QString const&) (ion.cpp:47)

176 (32 direct, 144 indirect) bytes in 1 blocks are definitely lost in loss 
record 882 of 1,156
   at 0x4C2596C: operator new(unsigned long) (vg_replace_malloc.c:220)
   by 0x1F267795: IonInterface::IonInterface(QObject*, QList<QVariant> const&) 
(ion.cpp:36)
   by 0x1FAE151D: WetterComIon::WetterComIon(QObject*, QList<QVariant> const&) 
(ion_wettercom.cpp:68)
   by 0x1FAEE366: QObject* KPluginFactory::createInstance<WetterComIon, 
QObject>(QWidget*, QObject*, QList<QVariant> const&) (kpluginfactory.h:461)
   by 0x5B3367B: KPluginFactory::create(char const*, QWidget*, QObject*, 
QList<QVariant> const&, QString const&) (kpluginfactory.cpp:191)
   by 0x5594EB5: Plasma::DataEngineManager::loadEngine(QString const&) 
(kpluginfactory.h:515)
   by 0x1F6C813F: WeatherEngine::loadIon(QString const&) (weatherengine.cpp:92)
   by 0x1F6C8C7C: WeatherEngine::sourceRequestEvent(QString const&) 
(weatherengine.cpp:226)
   by 0x55913AC: Plasma::DataEnginePrivate::requestSource(QString const&, 
bool*) (dataengine.cpp:670)
   by 0x5591421: Plasma::DataEngine::connectSource(QString const&, QObject*, 
unsigned int, Plasma::IntervalAlignment) const (dataengine.cpp:89)
   by 0x1F05608D: WeatherPopupApplet::connectToEngine() 
(weatherpopupapplet.cpp:234)
   by 0x1F05815E: WeatherPopupApplet::init() (weatherpopupapplet.cpp:223)


Thanks,

Thilo-Alexander

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to