Code review comment for lp:~seb128/phone-app/ringtone-in-gsettings

Revision history for this message
Sebastien Bacher (seb128) wrote :

> The cmake-foo is not strong with me, but I think you should use something like find_package here to pick up > the pkg-config file.

Thanks ... the new gsettings-qt (which features a .pc) was not available in saucy yet when I started and I though there was no .pc to use ... fixed that in r719

> This should only be "#include <QGSettings>". The pkgconfig file already points to the QGSettings directory, so that should work as soon as you pull in its cflags.

Thanks, that was a side-effect of not having the pkgconfig, fixed in r718

> schemas/com.ubuntu.touch.phone-app.gschema.xml.in:
> * the summary and description for incoming-call-sound and incoming-message-sound are the same

Fixed that one as well

> * the absolute paths are a bit weird - don't we have sound themes based on xdg dirs like we do for icons?

I've no strong opinion, I went with the current approch since it allows to set a file anywhere on disk (e.g a music from your xdg music dir), I'm not sure if you can do that with the theme spec.

I think the spec makes sense for system effects for example, maybe not in that "custom sound" case

« Back to merge proposal