Code review comment for lp:~ralsina/ubuntu-system-settings/notification-plugin

Revision history for this message
Roberto Alsina (ralsina) wrote :

> Thanks, there are some issues though, review comments inline

Answered those inline as well, except fr the QML which I am playing with.

> You use the "com.ubuntu.touch.notifications" schemas, what is shipping it?
> Don't you need a depends to be added or versionned (new gsettings-ubuntu-
> touch-schemas?)

Yes, there is a branch (see link above) that adds that schema in gsettings-ubuntu-touch-schemas. To test it you'd need to buld that, install it, then build this and try it.

To see it in action, you need to:

1) Install a few click apps that support push.

Since those are rare, you can change has_helper to true in line 134 of notification_manager.cpp and that will make this show all click apps.

2) Start system settings, open this page, play around with it, enable/disable a few

3) Start it again, see that it keeps the settings correctly.

>
> Can you also give some hints on how to test those changes?

« Back to merge proposal