Code review comment for lp:~schwann/ubuntu-system-settings/settings-pick-background-from-gallery

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

Thanks, I got it mostly working doing that:

- build/install https://code.launchpad.net/~ken-vandine/content-hub/handler_interface/+merge/182723 (merged back with trunk, ignoring the example conflict)
- start the content-hub-service
- export APP_ID=gallery-app; gallery-app
- system-settings background

Picking an image works and send it back to the capplet, the configuration is buggy though since the gsettings key should be an uri (e.g starting with file://...) and it's given a path

Otherwise some comments on the code:

* do we need all those __, it makes variable/code harder to read imho

* do we need the peer variable, it's used only once, it seems we could as well make 1 line of those 2

* background.pictureUri = imageUrl;

-> that should be "background.pictureUri = "file://" + imageUrl;" I guess, if gallery returns a path (or we need to change it from the gallery side to return an uri)

review: Needs Fixing

« Back to merge proposal