davidedmundson added a comment.
It's a big review :/
The general infrastructure and design makes sense.
I shall review the individual classes separately during the week.
INLINE COMMENTS
> CMakeLists.txt:3
> +if(BUILD_TESTING)
> + #add_subdirectory(autotests)
> +endif()
That's one way to avoid test failures.
> CMakeLists.txt:26
> +
> +set(HAVE_PULSEAUDIOQT ${KF5PulseAudioQt_FOUND})
> +configure_file(config-notificationmanager.h.cmake
> ${CMAKE_CURRENT_BINARY_DIR}/config-notificationmanager.h )
I don't know the status of this, but please make sure we don't depend on
something in review
> notificationgroupcollapsingproxymodel.cpp:126
> +
> + QPersistentModelIndex persistentIdx(idx);
> + if (expanded) {
It seems safer to take the pesistent index from the sourceModel
We're invalidating this layer repeatedly which could invalidate our persistent
index
> notificationgroupcollapsingproxymodel.cpp:133
> +
> + invalidateFilter();
> +
you shouldn't need this, the data is changing so it'll get re-evaluated
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D20265
To: broulik, #plasma
Cc: davidedmundson, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel,
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
