Review Request: Convert Plasma Calendar to use new KHolidays API

[Aaron Seigo]

-----------------------------------------------------------
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
> 
>