Merge lp://staging/~mterry/gsettings-ubuntu-touch-schemas/volume into lp://staging/gsettings-ubuntu-touch-schemas

Proposed by Michael Terry
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 32
Merged at revision: 28
Proposed branch: lp://staging/~mterry/gsettings-ubuntu-touch-schemas/volume
Merge into: lp://staging/gsettings-ubuntu-touch-schemas
Diff against target: 111 lines (+70/-0)
6 files modified
accountsservice/50-com.ubuntu.AccountsService.pkla (+13/-0)
accountsservice/Makefile.am (+7/-0)
accountsservice/com.ubuntu.AccountsService.Sound.xml (+22/-0)
accountsservice/com.ubuntu.AccountsService.policy (+25/-0)
accountsservice/com.ubuntu.touch.AccountsService.Sound.xml (+1/-0)
debian/accountsservice-ubuntu-schemas.install (+2/-0)
To merge this branch: bzr merge lp://staging/~mterry/gsettings-ubuntu-touch-schemas/volume
Reviewer Review Type Date Requested Status
Sebastien Bacher (community) Approve
PS Jenkins bot continuous-integration Approve
Iain Lane (community) Abstain
Ted Gould Pending
Review via email: mp+209158@code.staging.launchpad.net

Commit message

Add Muted and Volume properties for sharing with the greeter.

Description of the change

Add Muted and Volume properties for sharing with the greeter.

I put them under the touch namespace, because the other sound settings are there too. Doesn't bother me, but I could understand if you want to move them under the ubuntu namespace.

I also added some permission files to let the greeter read/set the values but not anyone else.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks Mike, seems fine to me, can you drop the "touch" namespace?

In retrospect using it was a mistake, we already started moving away from it by renaming the binaries, it would be good to not add new keys to it and just use the ubuntu naming.

Laney, does that make sense to you?

review: Needs Fixing
Revision history for this message
Iain Lane (laney) wrote :

Yes, but we'd have to update the in-progress telephony-service landing and add the schemas to it to rename the keys. I can do that if we decide we want to.

As for this landing, I already expressed my distaste on IRC, but I'll put it here for the record and abstain since I want to defer to others.

I don't like using settings schemas to synchronise state between different parts of the system. In my opinion the session and the greeter ought to be able to communicate things like this between themselves and not go via an intermediary.

And could you please try to keep merge proposals about one change only? The CLEANFILES fix is probably small enough but you also changed SecurityPrivacy here.

review: Abstain
Revision history for this message
Michael Terry (mterry) wrote :

Fine, I can take out the CLEANFILES fix and propose a separate branch. I can also take out the SecurityPrivacy change and propose it again after this lands.

I know you don't like this solution. I don't have a strong preference either way, I just want this fixed, and this is the current design spec. We're talking it out in bug 840777 a bit.

It's important to have an intermediary like this. The user session may be open or not. The greeter may be open or not. The backing store must live outside of HOME for permission & encrypted-home reasons. And it needs to be persistent over a reboot. AccountsService seems like the perfect place. And it stores similar related data about users already.

31. By Michael Terry

Undo unrelated or vaguely related changes

32. By Michael Terry

Remove touch namespace from new keys

Revision history for this message
Michael Terry (mterry) wrote :

OK, removed touch namespace from my new keys. I didn't change old keys. That can be a separate effort if we want to pursue it.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

Just triple-confirmed with John Lea that per-user volume that changes in the greeter is what design wants. Given that design, I think this is the most reasonable way to make that happen.

Seb, can you please re-review?

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

Thanks Michael. The "use accountsservice to share between session and greeter" is still being discussed/doesn't get consensus, but we agreed in the previous settings meeting to use that meanwhile, so let's not block on it.

I'm comment approving only for now. Laney, if you think that shouldn't go in please comment, otherwise I'm going to approve that a bit later today.

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

Ok, I'm approving. It might make sense to put that in a silo with the indicator update that's going to use it

33. By Michael Terry

Remove two other instances of 'touch', whoops

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches