Merge lp:~michihenning/unity-scopes-api/miscellaneous-fixes into lp:unity-scopes-api

Proposed by Michi Henning
Status: Merged
Approved by: Marcus Tomlinson
Approved revision: 278
Merged at revision: 284
Proposed branch: lp:~michihenning/unity-scopes-api/miscellaneous-fixes
Merge into: lp:unity-scopes-api
Prerequisite: lp:~marcustomlinson/unity-scopes-api/lp-1410125
Diff against target: 295 lines (+40/-24)
21 files modified
CMakeLists.txt (+10/-7)
HACKING (+1/-1)
demo/CMakeLists.txt (+3/-1)
demo/scopes/scope-A/CMakeLists.txt (+2/-0)
demo/scopes/scope-B/CMakeLists.txt (+1/-0)
demo/scopes/scope-C/CMakeLists.txt (+1/-0)
demo/scopes/scope-D/CMakeLists.txt (+1/-0)
demo/scopes/scope-N/CMakeLists.txt (+1/-0)
demo/scopes/scope-S/CMakeLists.txt (+1/-0)
demo/stand-alone/CMakeLists.txt (+3/-1)
doc/Doxyfile-devel.in (+1/-1)
include/unity/scopes/Category.h (+1/-1)
include/unity/scopes/internal/CategoryRegistry.h (+5/-3)
include/unity/scopes/internal/smartscopes/SSScopeObject.h (+3/-3)
src/scopes/internal/JsonCppNode.cpp (+1/-1)
src/scopes/internal/Logger.cpp (+1/-1)
src/scopes/internal/QueryCtrlImpl.cpp (+0/-1)
src/scopes/internal/ReplyImpl.cpp (+1/-1)
src/scopes/internal/ScopeBaseImpl.cpp (+1/-0)
src/scopes/internal/SearchMetadataImpl.cpp (+1/-1)
src/scopes/utility/internal/BufferedResultForwarderImpl.cpp (+1/-1)
To merge this branch: bzr merge lp:~michihenning/unity-scopes-api/miscellaneous-fixes
Reviewer Review Type Date Requested Status
Marcus Tomlinson (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+246387@code.launchpad.net

This proposal supersedes a proposal from 2015-01-14.

Commit message

Bunch of miscellaneous fixes:

- Added warning for non-virtual destructors.
- Warnings will disappear once Marcus's child_scopes branch is merged.
- Tidied up the way compiler flags are set in CMakeLists.txt for clarity.
- Added missing dependencies to demos, so we can't run with an out-of-date registry or scoperunner.
- Fixed a number of warnings when building the developer doc.
- Minor doc fixes.
- Minor stylistic code fixes.
- Added missing tmp_dir_initialized_ initializer to ScopeBaseImpl.
- Removed redundant lock in QueryCtrlImpl constructor.

Description of the change

These are fixes that I picked up from the branches that went on hold because of the ABI break, but that contained other unrelated fixes.

Main change is that we now get a warning for non-virtual base class constructors. The warnings will disappear once Marcus's child_scopes branch is merged. The other fixes are all minor, except for the missing tmp_dir_initialized_ initialiser.

Bunch of miscellaneous fixes:

- Added warning for non-virtual destructors.
- Tidied up the way compiler flags are set in CMakeLists.txt for clarity.
- Added missing dependencies to demos, so we can't run with an out-of-date registry or scoperunner.
- Fixed a number of warnings when building the developer doc.
- Minor doc fixes.
- Minor stylistic code fixes.
- Added missing tmp_dir_initialized_ initializer to ScopeBaseImpl.
- Removed redundant lock in QueryCtrlImpl constructor.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

341 - // futher buffering.

Ahh, no more futher buffering? :P

LGTM

review: Approve
279. By Michi Henning

Merged trunk.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-01-19 08:08:55 +0000
3+++ CMakeLists.txt 2015-01-20 04:20:36 +0000
4@@ -147,15 +147,18 @@
5 # setting of LINK_FLAGS below).
6 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fvisibility=default")
7
8-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -pedantic -Wall -Wextra")
9+set(C_AND_CXX_WARNINGS "-pedantic -Wall -Wextra")
10
11 # Some additional warnings not included by the general flags set above.
12-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wcast-align -Wcast-qual -Wctor-dtor-privacy -Wformat -Wold-style-cast -Wredundant-decls -Wswitch-default")
13-
14-# TODO: Once AbstractScopeBase gets its virtual destructor, turn on the line below (bug #1360266).
15-#set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor")
16-
17-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pedantic -Wall -Wextra") # Can't use -std=c++11 with C compiler
18+set(EXTRA_C_WARNINGS "-Wcast-align -Wcast-qual -Wformat -Wredundant-decls -Wswitch-default")
19+set(EXTRA_CXX_WARNINGS "-Wnon-virtual-dtor -Wctor-dtor-privacy -Wold-style-cast")
20+
21+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_AND_CXX_WARNINGS}")
22+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_C_WARNINGS}")
23+
24+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 ${C_AND_CXX_WARNINGS}")
25+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_C_WARNINGS}")
26+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CXX_WARNINGS}")
27
28 # -fno-permissive causes warnings with clang, so we only enable it for gcc
29 if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
30
31=== modified file 'HACKING'
32--- HACKING 2015-01-06 01:05:48 +0000
33+++ HACKING 2015-01-20 04:20:36 +0000
34@@ -193,7 +193,7 @@
35 Autopkg test suite
36 ------------------
37
38-Scopes comes with an autopkg smoke test.
39+Scopes come with an autopkg smoke test.
40
41 http://packaging.ubuntu.com/html/auto-pkg-test.html
42
43
44=== modified file 'demo/CMakeLists.txt'
45--- demo/CMakeLists.txt 2014-04-29 03:14:49 +0000
46+++ demo/CMakeLists.txt 2015-01-20 04:20:36 +0000
47@@ -9,7 +9,9 @@
48
49 add_executable(scopes-client client.cpp)
50 # Add_dependencies should be used sparingly. In this case we need the global
51-# header to be generated before we start building the client binary.
52+# header to be generated before we start building the client binary, and
53+# we need an up-to-date registry to run it.
54 add_dependencies(scopes-client globalheader)
55+add_dependencies(scopes-client scoperegistry)
56 target_link_libraries(scopes-client ${UNITY_SCOPES_LIB} ${OTHER_LIBS})
57 install(TARGETS scopes-client DESTINATION bin)
58
59=== modified file 'demo/scopes/scope-A/CMakeLists.txt'
60--- demo/scopes/scope-A/CMakeLists.txt 2014-06-17 00:53:21 +0000
61+++ demo/scopes/scope-A/CMakeLists.txt 2015-01-20 04:20:36 +0000
62@@ -4,7 +4,9 @@
63 add_library(scope-A MODULE scope-A.cpp)
64 # Add_dependencies should be used sparingly. In this case we need the global
65 # header to be generated before we start building the client binary.
66+# We also need the scoperegistry and scoperunner to be up-to-date.
67 add_dependencies(scope-A globalheader)
68+add_dependencies(scope-A scoperegistry scoperunner)
69 set_target_properties(scope-A PROPERTIES
70 LINK_FLAGS "${ldflags} -Wl,--version-script,${symbol_map}")
71 set_target_properties(scope-A PROPERTIES LINK_DEPENDS ${symbol_map})
72
73=== modified file 'demo/scopes/scope-B/CMakeLists.txt'
74--- demo/scopes/scope-B/CMakeLists.txt 2013-11-19 02:51:46 +0000
75+++ demo/scopes/scope-B/CMakeLists.txt 2015-01-20 04:20:36 +0000
76@@ -1,3 +1,4 @@
77 add_library(scope-B MODULE scope-B.cpp)
78+add_dependencies(scope-B scoperegistry scoperunner)
79
80 configure_file(scope-B.ini.in scope-B.ini)
81
82=== modified file 'demo/scopes/scope-C/CMakeLists.txt'
83--- demo/scopes/scope-C/CMakeLists.txt 2013-11-19 02:51:46 +0000
84+++ demo/scopes/scope-C/CMakeLists.txt 2015-01-20 04:20:36 +0000
85@@ -1,3 +1,4 @@
86 add_library(scope-C MODULE scope-C.cpp)
87+add_dependencies(scope-C scoperegistry scoperunner)
88
89 configure_file(scope-C.ini.in scope-C.ini)
90
91=== modified file 'demo/scopes/scope-D/CMakeLists.txt'
92--- demo/scopes/scope-D/CMakeLists.txt 2013-11-19 02:51:46 +0000
93+++ demo/scopes/scope-D/CMakeLists.txt 2015-01-20 04:20:36 +0000
94@@ -1,3 +1,4 @@
95 add_library(scope-D MODULE scope-D.cpp)
96+add_dependencies(scope-D scoperegistry scoperunner)
97
98 configure_file(scope-D.ini.in scope-D.ini)
99
100=== modified file 'demo/scopes/scope-N/CMakeLists.txt'
101--- demo/scopes/scope-N/CMakeLists.txt 2013-11-19 02:51:46 +0000
102+++ demo/scopes/scope-N/CMakeLists.txt 2015-01-20 04:20:36 +0000
103@@ -1,3 +1,4 @@
104 add_library(scope-N MODULE scope-N.cpp)
105+add_dependencies(scope-N scoperegistry scoperunner)
106
107 configure_file(scope-N.ini.in scope-N.ini)
108
109=== modified file 'demo/scopes/scope-S/CMakeLists.txt'
110--- demo/scopes/scope-S/CMakeLists.txt 2013-11-19 02:51:46 +0000
111+++ demo/scopes/scope-S/CMakeLists.txt 2015-01-20 04:20:36 +0000
112@@ -1,3 +1,4 @@
113 add_library(scope-S MODULE scope-S.cpp)
114+add_dependencies(scope-S scoperegistry scoperunner)
115
116 configure_file(scope-S.ini.in scope-S.ini)
117
118=== modified file 'demo/stand-alone/CMakeLists.txt'
119--- demo/stand-alone/CMakeLists.txt 2014-04-17 06:08:05 +0000
120+++ demo/stand-alone/CMakeLists.txt 2015-01-20 04:20:36 +0000
121@@ -3,7 +3,9 @@
122 add_executable(stand-alone-client stand-alone-client.cpp)
123
124 # Add_dependencies should be used sparingly. In this case we need the global
125-# header to be generated before we start building the client binary.
126+# header to be generated before we start building the client binary, and
127+# we need an up-to-date registry to run it.
128 add_dependencies(stand-alone-client globalheader)
129+add_dependencies(stand-alone-client scoperegistry)
130 add_definitions(-DDEMO_RUNTIME_PATH="${CMAKE_CURRENT_BINARY_DIR}/Runtime.ini")
131 target_link_libraries(stand-alone-client ${UNITY_SCOPES_LIB} ${OTHER_LIBS})
132
133=== modified file 'doc/Doxyfile-devel.in'
134--- doc/Doxyfile-devel.in 2014-06-16 03:26:21 +0000
135+++ doc/Doxyfile-devel.in 2015-01-20 04:20:36 +0000
136@@ -1758,7 +1758,7 @@
137 # DOT_GRAPH_MAX_NODES then the graph will not be shown at all. Also note
138 # that the size of a graph can be further restricted by MAX_DOT_GRAPH_DEPTH.
139
140-DOT_GRAPH_MAX_NODES = 50
141+DOT_GRAPH_MAX_NODES = 100
142
143 # The MAX_DOT_GRAPH_DEPTH tag can be used to set the maximum depth of the
144 # graphs generated by dot. A depth value of 3 means that only nodes reachable
145
146=== modified file 'include/unity/scopes/Category.h'
147--- include/unity/scopes/Category.h 2014-11-03 05:31:30 +0000
148+++ include/unity/scopes/Category.h 2015-01-20 04:20:36 +0000
149@@ -42,7 +42,7 @@
150 \brief A set of related results returned by a scope
151 and displayed within a single pane in the Unity dash.
152
153- To create a Category, use ReplyProxy::register_category.
154+ To create a Category, use SearchReplyProxy::register_category.
155 \see ResultItem
156 */
157 class Category
158
159=== modified file 'include/unity/scopes/internal/CategoryRegistry.h'
160--- include/unity/scopes/internal/CategoryRegistry.h 2014-11-03 05:31:30 +0000
161+++ include/unity/scopes/internal/CategoryRegistry.h 2015-01-20 04:20:36 +0000
162@@ -46,13 +46,15 @@
163 CategoryRegistry() = default;
164
165 /**
166- \brief Deserializes category from a variant_map and registers it. Throws if category with same id exists.
167+ \brief Deserializes category from a variant_map and registers it.
168+ \throws unity::InvalidArgumentException if a category with the same id exists already.
169 \return category instance
170 */
171 Category::SCPtr register_category(VariantMap const& variant_map);
172
173 /**
174- \brief Creates category from supplied parameters. Throws if category with same id exists.
175+ \brief Creates category from supplied parameters.
176+ \throws unity::InvalidArgumentException if a category with the same id exists already.
177 \return category instance
178 */
179 Category::SCPtr register_category(std::string const& id, std::string const& title, std::string const& icon, CannedQuery::SCPtr const& query, CategoryRenderer const& renderer_template);
180@@ -65,7 +67,7 @@
181
182 /**
183 \brief Register an existing category instance with this registry.
184- Throws if category with sane id exists.
185+ \throws unity::InvalidArgumentException if a category with the same id exists already.
186 */
187 void register_category(Category::SCPtr category);
188
189
190=== modified file 'include/unity/scopes/internal/smartscopes/SSScopeObject.h'
191--- include/unity/scopes/internal/smartscopes/SSScopeObject.h 2015-01-09 03:16:51 +0000
192+++ include/unity/scopes/internal/smartscopes/SSScopeObject.h 2015-01-20 04:20:36 +0000
193@@ -52,9 +52,9 @@
194
195 // Remote operation implementations
196 MWQueryCtrlProxy search(CannedQuery const& q,
197- SearchMetadata const& hints,
198- MWReplyProxy const& reply,
199- InvokeInfo const& info) override;
200+ SearchMetadata const& hints,
201+ MWReplyProxy const& reply,
202+ InvokeInfo const& info) override;
203
204 MWQueryCtrlProxy activate(Result const& result,
205 ActionMetadata const& hints,
206
207=== modified file 'src/scopes/internal/JsonCppNode.cpp'
208--- src/scopes/internal/JsonCppNode.cpp 2015-01-13 09:12:06 +0000
209+++ src/scopes/internal/JsonCppNode.cpp 2015-01-20 04:20:36 +0000
210@@ -281,7 +281,7 @@
211 return std::make_shared<JsonCppNode>(value_node);
212 }
213
214-JsonNodeInterface::SPtr JsonCppNode::get_node(uint node_index) const
215+JsonNodeInterface::SPtr JsonCppNode::get_node(unsigned int node_index) const
216 {
217 if (root_.type() != Json::arrayValue)
218 {
219
220=== modified file 'src/scopes/internal/Logger.cpp'
221--- src/scopes/internal/Logger.cpp 2015-01-18 23:48:55 +0000
222+++ src/scopes/internal/Logger.cpp 2015-01-20 04:20:36 +0000
223@@ -119,7 +119,7 @@
224 }
225 }
226
227-Logger::operator src::severity_channel_logger_mt<>&()
228+Logger::operator boost::log::sources::severity_channel_logger_mt<>&()
229 {
230 return logger_; // No lock needed, immutable
231 }
232
233=== modified file 'src/scopes/internal/QueryCtrlImpl.cpp'
234--- src/scopes/internal/QueryCtrlImpl.cpp 2015-01-09 03:16:51 +0000
235+++ src/scopes/internal/QueryCtrlImpl.cpp 2015-01-20 04:20:36 +0000
236@@ -47,7 +47,6 @@
237 // inform the reply object belonging to this query that the query is finished.
238 assert(reply_proxy);
239
240- lock_guard<mutex> lock(mutex_);
241 ready_ = ctrl_proxy != nullptr;
242 cancelled_ = false;
243 }
244
245=== modified file 'src/scopes/internal/ReplyImpl.cpp'
246--- src/scopes/internal/ReplyImpl.cpp 2015-01-19 00:28:17 +0000
247+++ src/scopes/internal/ReplyImpl.cpp 2015-01-20 04:20:36 +0000
248@@ -92,7 +92,7 @@
249 {
250 fwd()->finished(CompletionDetails(CompletionDetails::OK)); // Oneway, can't block
251 }
252- catch (std::exception const& e)
253+ catch (std::exception const&)
254 {
255 // No logging here because this may happen after the run time is destroyed.
256 }
257
258=== modified file 'src/scopes/internal/ScopeBaseImpl.cpp'
259--- src/scopes/internal/ScopeBaseImpl.cpp 2014-11-13 02:31:13 +0000
260+++ src/scopes/internal/ScopeBaseImpl.cpp 2015-01-20 04:20:36 +0000
261@@ -39,6 +39,7 @@
262 : scope_dir_initialized_(false)
263 , cache_dir_initialized_(false)
264 , app_dir_initialized_(false)
265+ , tmp_dir_initialized_(false)
266 , registry_initialized_(false)
267 , settings_db_initialized_(false)
268 {
269
270=== modified file 'src/scopes/internal/SearchMetadataImpl.cpp'
271--- src/scopes/internal/SearchMetadataImpl.cpp 2014-09-02 15:47:35 +0000
272+++ src/scopes/internal/SearchMetadataImpl.cpp 2015-01-20 04:20:36 +0000
273@@ -19,8 +19,8 @@
274 #include <unity/scopes/internal/SearchMetadataImpl.h>
275 #include <unity/scopes/internal/Utils.h>
276 #include <unity/scopes/ScopeExceptions.h>
277+
278 #include <unity/UnityExceptions.h>
279-#include <sstream>
280
281 namespace unity
282 {
283
284=== modified file 'src/scopes/utility/internal/BufferedResultForwarderImpl.cpp'
285--- src/scopes/utility/internal/BufferedResultForwarderImpl.cpp 2014-12-01 09:47:04 +0000
286+++ src/scopes/utility/internal/BufferedResultForwarderImpl.cpp 2015-01-20 04:20:36 +0000
287@@ -72,7 +72,7 @@
288 // scope author tells us that results for this forwarder are now ready
289 // to be displayed (or the query has finished); set the 'ready' flag
290 // and if previous forwarder is also ready, then push and disable
291- // futher buffering.
292+ // further buffering.
293 if (!ready_.exchange(true))
294 {
295 if (previous_ready_ || !has_previous_)

Subscribers

People subscribed via source and target branches

to all changes: