unity-scope-click

Merge lp://staging/~mvo/unity-scope-click/lp1292645-use-libclick2 into lp://staging/unity-scope-click/devel

  1. lp1292645-use-libclick2
  2. Merge into devel
Proposed by Michael Vogt
Status: Work in progress
Proposed branch: lp://staging/~mvo/unity-scope-click/lp1292645-use-libclick2
Merge into: lp://staging/unity-scope-click/devel
Diff against target: 988 lines (+447/-221)
14 files modified
debian/control (+1/-0)
debian/tests/control (+2/-0)
libclickscope/click/CMakeLists.txt (+3/-0)
scope/click/CMakeLists.txt (+9/-0)
scope/click/configuration.cpp (+2/-20)
scope/click/configuration.h (+1/-5)
scope/click/interface.cpp (+47/-56)
scope/click/interface.h (+8/-5)
scope/click/libclick.cpp (+123/-0)
scope/click/libclick.h (+58/-0)
scope/tests/CMakeLists.txt (+10/-0)
scope/tests/test_configuration.cpp (+8/-56)
scope/tests/test_interface.cpp (+91/-79)
scope/tests/test_libclick.cpp (+84/-0)
To merge this branch: bzr merge lp://staging/~mvo/unity-scope-click/lp1292645-use-libclick2
Reviewer Review Type Date Requested Status
dobey (community) Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+220433@code.staging.launchpad.net

Commit message

Use libclick to gather information about installed frameworks/installed apps. This fixes lp:1292645.

Description of the change

Another attempt to start using libclick instead of spawning the click commandline client.

This branch addresses the feedback from from Alejandro, Rodney and Thomas in https://code.launchpad.net/~mvo/unity-scope-click/lp1292645-use-libclick/+merge/220061 in a fresh branch as the changes are big compared to the original version.

Based on https://code.launchpad.net/~alecu/unity-scope-click/lp1292645 and lp:~mvo/unity-scope-click/lp1292645-use-libclick.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:270
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/77/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/52/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/51/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/51/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/77/rebuild

review: Needs Fixing (continuous-integration)
Reply
lp://staging/~mvo/unity-scope-click/lp1292645-use-libclick2 updated
271. By Michael Vogt

add missing click libs to libclickscope/click/

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:271
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/78/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/53/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/52/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/52/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/78/rebuild

review: Needs Fixing (continuous-integration)
Reply
lp://staging/~mvo/unity-scope-click/lp1292645-use-libclick2 updated
272. By Michael Vogt

add missing ubuntu-sdk-libs to test control

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:272
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/79/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/53/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/53/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/79/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
dobey (dobey) wrote :

Thanks for working on this Michael.

You need to update to the tip of /devel. The configuration.{cpp,h} and associated tests have been moved under libclickscope/ now, so you'll need to update and resolve the conflicts by getting your changes under that directory instead.

The new libclick.{cpp,h} and associated tests should be placed in libclickscope/ instead, as well.

Also, I've noticed this pattern in your changes:

210 + try {
211 + ManifestList manifests = manifest_list_from_json(manifests_json);
212 + callback(manifests, ManifestError::NoError);
213 + } catch (...) {
214 + callback(ManifestList(), ManifestError::ParseError);
215 + }

This can be problematic, as any exceptions thrown inside callback() will result in callback() being called a second time with different arguments. I think it would be better to do this instead:

ManifestList manifests;
try {
    manifests = manifest_list_from_json(manifests_json);
} catch (...) {
    callback(ManifestList(), ManifestError::ParseError);
    return;
}
callback(manifests, ManifestError::NoError);

This way callback() will only be called once, and any exceptions it throws won't be lost inside the try block.

review: Needs Fixing
Reply

Unmerged revisions

272. By Michael Vogt

add missing ubuntu-sdk-libs to test control

271. By Michael Vogt

add missing click libs to libclickscope/click/

270. By Michael Vogt

cleanup uneeded changes

269. By Michael Vogt

fixes lp:1292645

268. By Michael Vogt

remove Interface::run_process

267. By Michael Vogt

code cleanup

266. By Michael Vogt

make Interface::get_manifest_for_app() use libclick

265. By Michael Vogt

make Interface::get_manifests() use libclick

264. By Michael Vogt

Add new libclick.{cc,h} that wraps the vala/GObject parts of libclick we need

This is the first step towards fixing bug #1292645 by getting the list
of available frameworks via libclick.

This branch is based on the work of Alejandro J. Cura
 lp:~alecu/unity-scope-click/lp1292645

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

to all changes: