kdereview: events runner

[John Layt]

On 24 August 2010 22:06, Aaron J. Seigo <aseigo at kde.org> wrote:

> hi ..
>
> the events runner has been in kdereview for a while now and it is time to
> decide what to do with it :)
>
> Alexey: if there are no known issues with it, please move it into
> kdeplasma-
> addons/runners/
>
> kdeplasma-addons has no hard restrictions on coding style, but we do
> encourage
> the use of kdelibs style so that there is an ease of switching between
> plugins
> for all plasma developers. it is up to you, however.
>
> other than that, it would be a very nice feature to do something like:
> "events
> today" or "events tomorrow" and get a list of them back ...
>
> cheers :)
>
>
Interesting, haven't seen that before.  A few comments after a quick look, I
can get more specific later.

1) The runner directly interfaces with Akonadi, shouldn't the update parts
be moved into a Plasma Service? (Yes, I'm supposed to be looking at moving
the Calendar DataEngine into a Service but I'm doing fieldwork right now and
won't have time for a few more weeks).

2) It uses Boost shared pointers, shouldn't it be using a Qt equivalent?

3) The code is a bit messy, it needs some polishing to kdelibs standard.

4) I don't think variantToDateTime() and dateTimeToVariant() are needed
anymore, KDateTime is now a proper QMetaType so will work seamlessly with
QVariant.

5) The DateTimeRange and DateTimeParser classes have far more methods in
them than actually get used in Event, either trim back to only what is
needed, or look to move to kdelibs where it can be used in a general case,
but it would need a lot of work, e.g. data members are currently public and
accessed directly rather than via get methods.

6) The date/time input formats are not localised, i.e. are not what the user
expects to be able to enter.  You should try use the standard KLocale read
date/time functions, don't try reinvent the wheel.  If only a fixed format
is possible then ISO format would be preferred using KDateTime methods.

7) Have you tested timezones work properly?

In short, I don't think it's ready yet.  I'd be happy to work with Alexey in
a few weeks to use his code to develop a Service for this based around the
existing calendar DataEngine.

Cheers!

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100827/0d85cec1/attachment.htm