Merge lp://qastaging/~knitzsche/scope-aggregator/refactor-handle-methods_add-login_remove-shared-category into lp://qastaging/scope-aggregator
Status: | Merged |
---|---|
Approved by: | Kyle Nitzsche |
Approved revision: | 191 |
Merged at revision: | 174 |
Proposed branch: | lp://qastaging/~knitzsche/scope-aggregator/refactor-handle-methods_add-login_remove-shared-category |
Merge into: | lp://qastaging/scope-aggregator |
Diff against target: |
3725 lines (+1703/-1386) 15 files modified
DELETED_FEATURES.md (+1/-0) README.md (+270/-286) gtests/CMakeLists.txt (+4/-0) gtests/MockRegistry.h (+7/-0) gtests/scope_directory/login_1.json (+74/-0) gtests/scope_directory/login_2.json (+73/-0) gtests/scope_directory/login_3.json (+35/-0) gtests/scope_test.cpp (+110/-3) include/aggchildscope.h (+36/-1) include/query.h (+16/-9) src/CMakeLists.txt (+1/-0) src/aggchildscope.cpp (+34/-1) src/handle_results.cpp (+960/-0) src/query.cpp (+2/-9) src/utils.cpp (+80/-1077) |
To merge this branch: | bzr merge lp://qastaging/~knitzsche/scope-aggregator/refactor-handle-methods_add-login_remove-shared-category |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Zhang Enwei (community) | Approve | ||
Gary.Wang | Pending | ||
Review via email:
|
Description of the change
For a proposed 4.13 release.
* Significant refactor of the three methods that handle results:
handle_
were simply too hard to debug and understand, and they had redundant code to
lookup and, if needed, to register Categories. Each of the three methods is
refactored as follows: define an enum that states the possible run types for
the method: get state variables, analyze state and set run type, switch on run
type to select code. Category lookup and registration is now moved to new
set_result_
link to child is needed. The three methods (and related) are moved to a new
file for clarity: handle_results.cpp.
* Added support for a "login_renderer" and
"login_
the updated README.md. I also added gtests for this.
* Removed "shared category" from "keywords". This feature is not used, I think.
I did not find it in all child_scopes.json files in clicks branch. It is mostly
redundant with a "category". That is, they both confine keywords to a Category
(although category type also can include scopes). We can reimplemented any
features that were supported only in keywords that are not supported in
categories if needed.
* Added Query::
Note: With this MP, an agg scope child_scopes.json file does not need a
"departments" object: it's absence is sufficient to signal no departments.
* Updated/rewrote README.md.
I tested this in Today, News, Nearby and Photos. It would be awesome if the .so
could be tested with our other aggregators!
Note: I started doing this refactoring to help debug https:/ /bugs.launchpad .net/canonical- devices- system- image/+ bug/1622423. However, after the refactor that issue continues, and I suspect it may be a unity8 rendering issue (I am working with unity api team on this separately).