[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Convert Plasma Calendar to use new KHolidays API
From:       "Aaron Seigo" <aseigo () kde ! org>
Date:       2010-05-20 20:54:31
Message-ID: 20100520205431.14793.16476 () localhost
[Download RAW message or body]


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

Ship it!


ah, beautiful. i -hated- that code in the calendartable, but there wasn't much choice \
that i could find elsewhere. this is a very nice improvement indeed.

- Aaron


On 2010-05-20 20:20:08, John Layt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4079/
> -----------------------------------------------------------
> 
> (Updated 2010-05-20 20:20:08)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> In SC 4.5 KHolidays has a number of new API calls to provide more metadata on each \
> Holiday Region, such as Name, Country (including subdivisions such as States and \
> Provinces) and Language, to test validity of a given Holiday Region code, and to \
> return all holidays in a date range. 
> This patch adapts the plasma calendar data engine and calendar plasmoid to use this \
> new API to improve efficiency and usability.  However, because of the data engine \
> abstraction layer what is a small and simple change in other KHolidays clients \
> required major changes in the data engine to basically recreate and wrap the new \
> KHolidays API calls.  As such it could be argued that this is a new feature rather \
> than just an efficiency and usability fix, and so has missed the feature freeze \
> boat.  I'll leave that call to others, but if deemed too big I'll try a smaller \
> efficiency fix within the bounds of the existing data engine API. 
> * Efficiency is improved by fetching all the holidays to display in a single call, \
> rather than 3 calls by the plasmoid and 90 calls by the data engine (and thus \
> saving reading the holiday file 90 times), and by using the isValid(regionCode) API \
> instead of fetching all the regions and scanning for a match. 
> * Usability is improved by displaying the real name of the Holiday Region (e.g. if \
> it's actually a state or province) and the language each Holiday Region is \
> available in, and by including the language as a criteria in the default Holiday \
> Region selection so the user is more likely to get a useful Holiday Region (e.g. in \
> bilingual countries). 
> I've also included a few bug fixes and validity checking of the contents of the \
> data queries. 
> 
> Diffs
> -----
> 
> trunk/KDE/kdebase/workspace/libs/plasmaclock/calendartable.cpp 1128950 
> trunk/KDE/kdebase/workspace/plasma/generic/dataengines/calendar/calendarengine.h \
> 1128950  trunk/KDE/kdebase/workspace/plasma/generic/dataengines/calendar/calendarengine.cpp \
> 1128950  
> Diff: http://reviewboard.kde.org/r/4079/diff
> 
> 
> Testing
> -------
> 
> Extensive testing using plasmoidviewer, especially of the new default region \
> algorithm with various combinations of country and language. 
> 
> Thanks,
> 
> John
> 
> 

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


[prev in list] [next in list] [prev in thread] [next in thread]