Code review comment for lp:~tiagosh/gsettings-ubuntu-touch-schemas/add-call-messages-vibration-properties

Revision history for this message
Tiago Salem Herrmann (tiagosh) wrote :

> Looks fine to me. Having bools for those is fine, we could also have an int
> value (0=don't vibrate, 1=vibrate on ring, 2=vibrate on silence). Small
> wording comment inline as well

I think this is also possible, but as Lars said, the booleans allow combinations and match the UI requirements: https://wiki.ubuntu.com/Sound#Settings
Also as reference, this is the telephony-service patch to use the proposed approach with booleans: https://code.launchpad.net/~tiagosh/telephony-service/vibrate-on-calls-and-messages/+merge/225886

>
> Setting as "approve" by comment but not changing the status, it would be nice
> if an english speaker could comment on the wording issue. Otherwise that looks
> good so feel free to ack it once that's sorted out

I think you are right, but as I was asked to drop the gsettings part, we don't need the descriptions anymore.

« Back to merge proposal