[Differential] [Updated] D1722: Import next-gen libtaskmanager.

[hein (Eike Hein)]

hein marked 21 inline comments as done.
hein added inline comments.

INLINE COMMENTS

> hein wrote in abstracttasksmodel.h:63
> It means what the comment says: If it's true, the task is not subject to grouping. That means it will never be grouped. It's used by the applet's Don't allow/Allow This Program To Be Grouped action.

I improved the comment.

> broulik wrote in abstracttasksmodel.h:75
> Qt uses FullScreen capitalization usually.

Changed.

> broulik wrote in abstracttasksmodel.h:87
> nullptr

Done.

> broulik wrote in abstracttasksmodel.h:90
> override instead of virtual

Done.

> davidedmundson wrote in activityinfo.cpp:55
> ..and set pointer to null
> 
> Otherwise you have a crash if you delete them all and then then create a new one.
> 
> (or use QWeak and QShared pointer )

Done, also in TasksModel.

> hein wrote in activityinfo.cpp:69
> Good catch. I think the old lib had this issue too ...
> 
> KActivities has no convenient API to listen to changes in the list of running activities, it seems - instead you have to keep a KActivities::Info around for all activtities, and listen to their stateChanged signals. Annoying ...

Implemented.

> broulik wrote in launchertasksmodel.cpp:126
> An empty QUrl is invalid already but I understand you don't trust QUrl :)

Changed it ...

> broulik wrote in launchertasksmodel.h:61
> Either virtual or override (preferrably the latter)

Done.

> davidedmundson wrote in startuptasksmodel.cpp:146
> q->index(row)

Done.

> broulik wrote in startuptasksmodel.cpp:64
> QLatin1String

Done.

> broulik wrote in startuptasksmodel.cpp:182
> I prefer isEmpty(), also consistent with isNull()

Done.

> broulik wrote in startuptasksmodel.cpp:196
> .at(0)? operator[] is weird

Done.

> broulik wrote in startuptasksmodel.cpp:238
> QStringLiteral/QLatin1String

Done.

> broulik wrote in startuptasksmodel.cpp:242
> QLatin1String

Done.

> broulik wrote in taskfilterproxymodel.h:124
> ie. greater than -1

Superceded by David's review, fixed.

> broulik wrote in taskgroupingproxymodel.cpp:269
> reserve()?

Done.

> broulik wrote in taskgroupingproxymodel.cpp:270
> Use Initializer list, also in other places below

Done.

> davidedmundson wrote in xwindowtasksmodel.cpp:95
> windowInfoCache values leak in destructor

Done.

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/b84b482d/attachment-0001.html>