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
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
