<div>davidedmundson added inline comments.</div><br /><div><strong>INLINE COMMENTS</strong><div><div>imports/activitymanager/sortedactivitiesmodel.cpp:228 We should cache the config as a member var and only reparse when in ::currentActivity changes. Otherwise you're parsing an entire config file (in fact 3, see last comment) lots of times.<br />
<br />
If this gets used by a QWidget API that hammers data() on every repaint. If this is always purely QtQuick it's generally OK as the delegate's have to internally cache - but it's still once per activity rather than just once.<br />
<br />
for a speedup open this to SimpleConfig. That saves it also loading in kglobals and the rest of the cascading.</div><div>imports/activitymanager/sortedactivitiesmodel.cpp:239 Yep, you're right. <br />
For the time order to have changed, current activity would have changed dataChanged(). <br />
<br />
This is absolutely fine then.</div><div>imports/activitymanager/sortedactivitiesmodel.cpp:354 should check it's >= 0 (in both cases)</div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMADESKTOP Plasma Desktop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D986" rel="noreferrer">https://phabricator.kde.org/D986</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>ivan, davidedmundson, sebas, mart<br /><strong>Cc: </strong>plasma-devel<br /></div>