[Differential] [Commented On] D1722: Import next-gen libtaskmanager.

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> TODO.txt:1
> +Larger outstanding tasks:
> +- Implement missing kwayland bits (e.g. transient handling, virtual desktop logic).

do you really want to add a TODO.txt? My suggestion would be to add tasks to maniphest here

> abstracttasksmodel.cpp:33-35
> +AbstractTasksModel::~AbstractTasksModel()
> +{
> +}

AbstractTaskModel::~AbstractTaskModel() = default;

> abstracttasksmodel.h:92
> +
> +    virtual QModelIndex	index(int row, int column = 0, const QModelIndex &parent = QModelIndex()) const;
> +

This looks weird. There is  a virtual method QAbstractItemModel::index which has column not optional. So you are maybe hiding a virtual method. I suggest to use override here and make column not optional.

> abstracttasksmodel.h:94-102
> +    /**
> +     * Request activation of the task at the given index. Derived classes are
> +     * free to interpret the meaning of "activate" themselves depending on
> +     * the nature and state of the task, e.g. launch or raise a window task.
> +     *
> +     * This base implementation does nothing.
> +     *

why copy all the documentation? It's already documented in AbstractTasksModelIface. That yells like "will diverge" to me ;-)

> abstracttasksmodel.h:103
> +     **/
> +    virtual void requestActivate(const QModelIndex &index);
> +

please use override for these methods as well. They are defined in AbstractTasksModelIface after all.

> abstracttasksmodeliface.h:2
> +/********************************************************************
> +Copyright 2016  Eike Hein <hein.org>
> +

your email address in the copy right headers is not a valid email address (seems to be consistent over all header files)

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D1722

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: hein, Plasma
Cc: graesslin, broulik, davidedmundson, plasma-devel, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160601/75f9a48f/attachment-0001.html>