broulik added inline comments. INLINE COMMENTS
> davidedmundson wrote in jobsmodel_p.cpp:358 > you're not filtering the jobs based on the desktopEntry > > every app gets notified about anyones percentage? Good catch, forgot a check for `desktopEntry` there > hein wrote in notificationgroupingproxymodel.cpp:38 > Careful there - these will match when both are empty, which is why the libtm > version tests for that. Are you sure this data cannot ever be missing? I'm pretty sure it can never be empty as if all else fails I use the binary name of the process that sent the notification dbus message (which might fail after all?), I'll add a check nonetheless. > hein wrote in notifications.cpp:828 > While this isn't an objection, I usually use QMetaEnum to compute this from > the enum instead of having a block of stuff that needs to be synced. There's a gazillion places that need to be synced when we add something: - add enum in header - add new field to struct that holds the actual data - let `data()` return the new role as well as actually forwarding it in a bunch of places in QML, so I don't think automating this particular part really reduces the workload for that. I can give it a shot, though. > hein wrote in notifications.h:211 > I recommend making this Q_ENUM and registering it; with libtm it turned out > that it's useful to be able to call data() with a role from QML sometimes. I did exactly that in a later commit that isn't on Phab, sorry :) > hein wrote in notifications.h:217 > This is a bit unusual, I assume it's because you're flattening back out so > you don't want rowCount/canFetchMore? I guess. I use it for showing an expand button with "show n more" written on it REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D20265 To: broulik, #plasma Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
