indicator-power

Merge lp://staging/~albaguirre/indicator-power/use-new-brightness-dbus-interface into lp://staging/indicator-power/14.10

use-new-brightness-dbus-interface
Merge into trunk.14.10

Proposed by Alberto Aguirre on 2014-06-17

Status:	Merged
Approved by:	Lars Karlitski on 2014-06-24
Approved revision:	252
Merged at revision:	247
Proposed branch:	lp://staging/~albaguirre/indicator-power/use-new-brightness-dbus-interface
Merge into:	lp://staging/indicator-power/14.10
Diff against target:	334 lines (+71/-50) 6 files modified debian/control (+2/-0) m4/gcov.m4 (+1/-1) src/Makefile.am (+2/-2) src/ib-brightness-uscreen-control.c (+41/-22) src/ib-brightness-uscreen-control.h (+11/-11) src/service.c (+14/-14)
To merge this branch:	bzr merge lp://staging/~albaguirre/indicator-power/use-new-brightness-dbus-interface
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Lars Karlitski (community)		2014-06-17	Approve on 2014-06-24
PS Jenkins bot (community)	continuous-integration		Approve on 2014-06-20
Iain Lane (community)			Approve on 2014-06-20
Review via email: mp+223339@code.staging.launchpad.net

Commit message

Changes to address setBrightness interface moving from powerd to unity-system-compositor

Description of the change

Changes to address setBrightness interface moving from powerd to unity-system-compositor

Depends on:
https://code.launchpad.net/~albaguirre/unity-system-compositor/screen-power-state-handling/+merge/213957
https://code.launchpad.net/~albaguirre/powerd/remove-input-inactivity-handling/+merge/219449

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-17:

FAILED: Continuous integration, rev:247
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/105/
Executed test runs:
FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/6/console
FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/6/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/105/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Iain Lane (laney) wrote on 2014-06-19:

Cheers. Since you asked me to review I've taken a look, but Charles I think usually takes care of i-power. One inline comment too.

The main problem is that you need to rename the 'powerd' references everywhere since that name is now inaccurate except for getBrightnessParams.

What provides this service? Please either add a dependency or (maybe this is a good idea anyway) keep the fallback to powerd if nothing has the new name.

review: Needs Fixing

Revision history for this message

Charles Kerr (charlesk) wrote on 2014-06-19:

alberto, laney, IMO we should remove the brightness code from indicator-power altogether. IMO it doesn't make sense for i-power to maintain a feature that it's not using.

Revision history for this message

Charles Kerr (charlesk) wrote on 2014-06-19:

In the 50 minutes since my last comment, it looks like Design has decided to restore the slider to indicator-power after all -- this MR brought the discussion to a head. :)

After this MR goes through as an interim step, I'll revert the previous code that disabled the slider.

lp://staging/~albaguirre/indicator-power/use-new-brightness-dbus-interface updated on 2014-06-19

248. By Alberto Aguirre on 2014-06-19: Remove references to powerd since the brightness interface is now provided by Unity.Screen
249. By Alberto Aguirre on 2014-06-19: Remove powerd reference in message
250. By Alberto Aguirre on 2014-06-19: Add powerd and unity-system-compositor to Depends