Merge lp://staging/~charlesk/indicator-power/custom-bus-for-upower into lp://staging/indicator-power/15.04

Proposed by Charles Kerr
Status: Approved
Approved by: Ted Gould
Approved revision: 283
Proposed branch: lp://staging/~charlesk/indicator-power/custom-bus-for-upower
Merge into: lp://staging/indicator-power/15.04
Diff against target: 1005 lines (+556/-176)
7 files modified
README (+11/-0)
src/CMakeLists.txt (+1/-0)
src/brightness.c (+71/-30)
src/bus.c (+195/-0)
src/bus.h (+50/-0)
src/device-provider-upower.c (+67/-23)
src/service.c (+161/-123)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-power/custom-bus-for-upower
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+246234@code.staging.launchpad.net

Commit message

Add support for finer-grained dbus service mocking

Description of the change

== Change Description

Add support for finer-grained dbus service mocking.

Use case: say we want to feed the indicator a dbusmocked upower, but don't want to mock the entire system bus because we also talk to powerd on the system bus. This lets us specify via environment variables where to look for each bus dependency.

== Checklist

> Are there any related MPs required for this MP to build/function as expected? Please list.

No

> Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Yes

> Did the code build without warnings?

Yes

> Did the tests run successfully?

Yes

> Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> Did your component test plan pass? If on a device, what image number?

15.03 nexus 4 r63

> Please list which manual tests are germane for the reviewer in this MR.

This is QA scaffolding; no manual tests affected

> Did you provide a link to this page https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-power

Yes

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
Ted Gould (ted) wrote :

Have to say I'm a little nervous with abstracting the bus stuff out this much. I like the standardization of using g_bus_own_name() to setup the ordering of the callbacks and ensure that we're doing everything right there. Naturually, that loses some flexibility. I like sitting outside the process and using dbusmock better so that it can be the same name, on the bus configured to be DBUS_SYSTEM_BUS_ADDRESS so that the indicator can be simpler.

That said, I don't see anything wrong with this code and have no objection other than "not the way I would have done it."

review: Approve
Revision history for this message
Allan LeSage (allanlesage) wrote :

Getting requests for this awesome new feature already (from paulliu), let's land 'er!

Unmerged revisions

283. By Charles Kerr

pass the G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT flag when calling g_dbus_connection_new_for_address() on an arbitrary bus address

282. By Charles Kerr

when connecting to a custom bus address, use G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION

281. By Charles Kerr

document the debugging environment variables

280. By Charles Kerr

move indicator_power_get_screen_busname() to bus.c

279. By Charles Kerr

use indicator_power_get_bus() for acquiring the bus that we look for powerd on

278. By Charles Kerr

use indicator_power_get_bus() for acquiring the bus that we look for upower on

277. By Charles Kerr

use indicator_power_get_bus() for acquiring the indicator's bus

276. By Charles Kerr

add indicator_power_get_bus() which lets environment variables let a custom bus override other buses.

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