Merge lp://staging/~ted/ubuntu-app-launch/zg-ordering into lp://staging/ubuntu-app-launch

Proposed by Ted Gould
Status: Merged
Approved by: Ted Gould
Approved revision: 291
Merged at revision: 292
Proposed branch: lp://staging/~ted/ubuntu-app-launch/zg-ordering
Merge into: lp://staging/ubuntu-app-launch
Diff against target: 1355 lines (+598/-124)
28 files modified
.bzrignore (+1/-0)
libubuntu-app-launch/CMakeLists.txt (+4/-0)
libubuntu-app-launch/application-impl-base.cpp (+37/-0)
libubuntu-app-launch/application-impl-base.h (+3/-0)
libubuntu-app-launch/application-impl-click.cpp (+7/-2)
libubuntu-app-launch/application-impl-click.h (+2/-0)
libubuntu-app-launch/application-impl-legacy.cpp (+6/-1)
libubuntu-app-launch/application-impl-legacy.h (+2/-0)
libubuntu-app-launch/application-impl-libertine.cpp (+6/-1)
libubuntu-app-launch/application-impl-libertine.h (+2/-0)
libubuntu-app-launch/application-impl-snap.cpp (+57/-51)
libubuntu-app-launch/application-impl-snap.h (+2/-0)
libubuntu-app-launch/application-info-desktop.cpp (+8/-1)
libubuntu-app-launch/application-info-desktop.h (+7/-1)
libubuntu-app-launch/application.cpp (+1/-0)
libubuntu-app-launch/application.h (+6/-0)
libubuntu-app-launch/info-watcher-zg.cpp (+45/-0)
libubuntu-app-launch/info-watcher-zg.h (+47/-0)
libubuntu-app-launch/info-watcher.cpp (+35/-0)
libubuntu-app-launch/info-watcher.h (+52/-0)
libubuntu-app-launch/registry-impl.cpp (+20/-1)
libubuntu-app-launch/registry-impl.h (+23/-0)
libubuntu-app-launch/registry.cpp (+5/-0)
libubuntu-app-launch/registry.h (+10/-0)
tests/CMakeLists.txt (+10/-1)
tests/application-info-desktop.cpp (+121/-65)
tests/info-watcher-zg.cpp (+55/-0)
tests/registry-mock.h (+24/-0)
To merge this branch: bzr merge lp://staging/~ted/ubuntu-app-launch/zg-ordering
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Abstain
Charles Kerr (community) Approve
unity-api-1-bot continuous-integration Needs Fixing
Review via email: mp+316967@code.staging.launchpad.net

Commit message

Provide a popularity info item and a signal for info updating

Description of the change

This branch sets up the framework for updating info based on all kinds of things, one of which is the popularity of the package. Mostly it provides just empty classes, but it strings them all together to show that they could work together. It also adjusts the API in the needed ways.

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote :

there's a typo: Zietgeist -> Zeitgeist

review: Needs Fixing
289. By Ted Gould

Move to the code slightly so that it doesn't conflict

290. By Ted Gould

s/Zietgeist/Zeitgeist/

291. By Ted Gould

Make sure we have a default value for the popularity call

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Mixed feelings about this branch.

On the one hand, this adds new brittle API -- I think out of necessity to work with previous API choices -- and this branch makes the code even more complex than it already is. One example is our the registry impl internals in tests because of how difficult it is to do dependency injection when relying heavily on static methods in a class heierarchy. But that's just one example.

OTOH: 1. the fix does what it says on the tin, 2. there's no outright NF code, 3. fixing complexity is out of scope of this patch.

Approved, but I feel like this code is headed in the wrong direction. We need to think about how to simplify UAL's code overall.

review: Approve
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Typo fixed. It seems to suit my needs. Abstaining as I don't have too much insight on u-a-l internals otherwise.

review: Abstain

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