Mir

Merge lp:~afrantzis/mir/platform-operation-client-api-remove-opcode into lp:mir

  1. platform-operation-client-api-remove-opcode
  2. Merge into development-branch
Proposed by Alexandros Frantzis on 2015-02-03
Status: Merged
Approved by: Alan Griffiths on 2015-02-03
Approved revision: 2282
Merged at revision: 2288
Proposed branch: lp:~afrantzis/mir/platform-operation-client-api-remove-opcode
Merge into: lp:mir
Prerequisite: lp:~afrantzis/mir/platform-operation-set-gbm-device
Diff against target: 163 lines (+10/-19)
8 files modified
client-ABI-sha1sums (+1/-1)
include/client/mir_toolkit/mir_connection.h (+1/-2)
src/client/mir_connection.cpp (+1/-2)
src/client/mir_connection.h (+0/-5)
src/client/mir_connection_api.cpp (+2/-4)
src/server/graphics/nested/mir_client_host_connection.cpp (+1/-1)
tests/acceptance-tests/test_client_platform_operation.cpp (+2/-2)
tests/unit-tests/client/test_mir_connection.cpp (+2/-2)
To merge this branch: bzr merge lp:~afrantzis/mir/platform-operation-client-api-remove-opcode
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration 2015-02-03 Approve on 2015-02-03
Robert Carr (community) 2015-02-03 Approve on 2015-02-03
Alan Griffiths Approve on 2015-02-03
Daniel van Vugt 2015-02-03 Pending
Chris Halse Rogers 2015-02-03 Pending
Kevin DuBois 2015-02-03 Pending
Review via email: mp+248338@code.launchpad.net

This proposal supersedes a proposal from 2015-02-02.

Commit Message

client: Remove redundant opcode parameter from platform operation API

Description of the Change

client: Remove redundant opcode parameter from platform operation API

During development/refactorings of the client platform operation API the redundancy of the opcode parameter (which is a mandatory parameter of a MirPlatformMessage) went unnoticed.

This is a client API/ABI break, but since the platform operation API is not currently used by external code (and could not have been used because we didn't expose any operations), we can get away with it.

To post a comment you must log in.
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

LGTM

review: Approve
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I approve but we need more eyes on this given we're knowingly breaking part of the client API that's been around a long time.

review: Needs Information
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:2282
http://jenkins.qa.ubuntu.com/job/mir-ci/2875/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1155
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/872
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/872/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1115
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1115/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4168
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17674

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2875/rebuild

review: Approve (continuous-integration)
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

lgtm

review: Approve
Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

LGTM

review: Approve
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Targeting the wrong branch now. Somehow!?

review: Resubmit
Alan Griffiths (alan-griffiths) wrote :

LGTM

review: Approve
Robert Carr (robertcarr) :
review: Approve
Robert Carr (robertcarr) wrote :

TAing due to previous approvals before resubmit.

PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2282
http://jenkins.qa.ubuntu.com/job/mir-ci/2877/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1159
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1119
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/874
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/874/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1119
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1119/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4172
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17699

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2877/rebuild

review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'client-ABI-sha1sums'
2--- client-ABI-sha1sums 2015-01-29 04:34:12 +0000
3+++ client-ABI-sha1sums 2015-02-03 09:35:55 +0000
4@@ -1,7 +1,7 @@
5 afd66e984f4f8c0303fd25fb6fea32fa47bbd37f include/client/mir_toolkit/debug/surface.h
6 b53736bcb22ddc09aab8275cfdd05c109fdd0f12 include/client/mir_toolkit/mir_client_library_drm.h
7 532af89e0f48d8d2a7cf01621db25d3338ae3abf include/client/mir_toolkit/mir_client_library.h
8-750aa35e6a5173bcba47cb524c508d05a4df746c include/client/mir_toolkit/mir_connection.h
9+615df675ffba805fb0594e19fb006f514f2100a7 include/client/mir_toolkit/mir_connection.h
10 1ef8f51a3e3f8d1559266c5af58fbfde7cfabf0a include/client/mir_toolkit/mir_cursor_configuration.h
11 d739af6e64db6b314a727b5fb00be662b98ccd57 include/client/mir_toolkit/mir_platform_message.h
12 9d50df5a141ca03ee8a79f7e844ed4b8b3b7d5d3 include/client/mir_toolkit/mir_prompt_session.h
13
14=== modified file 'include/client/mir_toolkit/mir_connection.h'
15--- include/client/mir_toolkit/mir_connection.h 2015-01-14 06:39:13 +0000
16+++ include/client/mir_toolkit/mir_connection.h 2015-02-03 09:35:55 +0000
17@@ -178,14 +178,13 @@
18 * until this operation finishes.
19 *
20 * \param [in] connection The connection
21- * \param [in] opcode The opcode of the operation to perform
22 * \param [in] request The message used for this operation
23 * \param [in] callback The callback to call when the operation finishes
24 * \param [in,out] context User data passed to the callback function
25 * \return A handle that can be passed to mir_wait_for
26 */
27 MirWaitHandle* mir_connection_platform_operation(
28- MirConnection* connection, unsigned int opcode,
29+ MirConnection* connection,
30 MirPlatformMessage const* request,
31 mir_platform_operation_callback callback, void* context);
32
33
34=== modified file 'src/client/mir_connection.cpp'
35--- src/client/mir_connection.cpp 2015-02-03 09:35:55 +0000
36+++ src/client/mir_connection.cpp 2015-02-03 09:35:55 +0000
37@@ -357,7 +357,6 @@
38 }
39
40 MirWaitHandle* MirConnection::platform_operation(
41- unsigned int opcode,
42 MirPlatformMessage const* request,
43 mir_platform_operation_callback callback, void* context)
44 {
45@@ -371,7 +370,7 @@
46
47 mir::protobuf::PlatformOperationMessage protobuf_request;
48
49- protobuf_request.set_opcode(opcode);
50+ protobuf_request.set_opcode(mir_platform_message_get_opcode(request));
51 auto const request_data = mir_platform_message_get_data(request);
52 auto const request_fds = mir_platform_message_get_fds(request);
53
54
55=== modified file 'src/client/mir_connection.h'
56--- src/client/mir_connection.h 2015-02-03 09:35:55 +0000
57+++ src/client/mir_connection.h 2015-02-03 09:35:55 +0000
58@@ -107,12 +107,7 @@
59
60 MirWaitHandle* disconnect();
61
62- MirWaitHandle* drm_auth_magic(unsigned int magic,
63- mir_drm_auth_magic_callback callback,
64- void* context);
65-
66 MirWaitHandle* platform_operation(
67- unsigned int opcode,
68 MirPlatformMessage const* request,
69 mir_platform_operation_callback callback, void* context);
70
71
72=== modified file 'src/client/mir_connection_api.cpp'
73--- src/client/mir_connection_api.cpp 2015-02-03 09:35:55 +0000
74+++ src/client/mir_connection_api.cpp 2015-02-03 09:35:55 +0000
75@@ -306,13 +306,13 @@
76 }
77
78 MirWaitHandle* mir_connection_platform_operation(
79- MirConnection* connection, unsigned int opcode,
80+ MirConnection* connection,
81 MirPlatformMessage const* request,
82 mir_platform_operation_callback callback, void* context)
83 {
84 try
85 {
86- return connection->platform_operation(opcode, request, callback, context);
87+ return connection->platform_operation(request, callback, context);
88 }
89 catch (std::exception const& ex)
90 {
91@@ -387,7 +387,6 @@
92
93 return mir_connection_platform_operation(
94 connection,
95- MirMesaPlatformOperation::auth_magic,
96 msg.get(),
97 platform_operation_to_auth_magic_callback,
98 auth_magic_op_context);
99@@ -409,7 +408,6 @@
100
101 auto wh = mir_connection_platform_operation(
102 connection,
103- MirMesaPlatformOperation::set_gbm_device,
104 msg.get(),
105 assign_set_gbm_device_status,
106 &status);
107
108=== modified file 'src/server/graphics/nested/mir_client_host_connection.cpp'
109--- src/server/graphics/nested/mir_client_host_connection.cpp 2015-01-26 15:53:18 +0000
110+++ src/server/graphics/nested/mir_client_host_connection.cpp 2015-02-03 09:35:55 +0000
111@@ -185,7 +185,7 @@
112 MirPlatformMessage* raw_reply{nullptr};
113
114 auto const wh = mir_connection_platform_operation(
115- mir_connection, op, msg.get(), platform_operation_callback, &raw_reply);
116+ mir_connection, msg.get(), platform_operation_callback, &raw_reply);
117 mir_wait_for(wh);
118
119 auto const reply = mir::raii::deleter_for(
120
121=== modified file 'tests/acceptance-tests/test_client_platform_operation.cpp'
122--- tests/acceptance-tests/test_client_platform_operation.cpp 2014-12-11 08:54:04 +0000
123+++ tests/acceptance-tests/test_client_platform_operation.cpp 2015-02-03 09:35:55 +0000
124@@ -39,7 +39,7 @@
125 MirPlatformMessage* reply;
126
127 auto const platform_op_done = mir_connection_platform_operation(
128- connection, add_opcode, request, assign_reply, &reply);
129+ connection, request, assign_reply, &reply);
130 mir_wait_for(platform_op_done);
131
132 mir_platform_message_release(request);
133@@ -57,7 +57,7 @@
134 MirPlatformMessage* reply;
135
136 auto const platform_op_done = mir_connection_platform_operation(
137- connection, echo_fd_opcode, request, assign_reply, &reply);
138+ connection, request, assign_reply, &reply);
139 mir_wait_for(platform_op_done);
140
141 mir_platform_message_release(request);
142
143=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
144--- tests/unit-tests/client/test_mir_connection.cpp 2015-02-03 09:35:55 +0000
145+++ tests/unit-tests/client/test_mir_connection.cpp 2015-02-03 09:35:55 +0000
146@@ -615,7 +615,7 @@
147 MirPlatformMessage* returned_response{nullptr};
148
149 auto op_wh = connection->platform_operation(
150- opcode, request.get(), assign_response, &returned_response);
151+ request.get(), assign_response, &returned_response);
152 mir_wait_for(op_wh);
153
154 EXPECT_THAT(returned_response, Eq(response.get()));
155@@ -642,7 +642,7 @@
156 MirPlatformMessage* returned_response{nullptr};
157
158 auto op_wh = connection->platform_operation(
159- opcode, request.get(), assign_response, &returned_response);
160+ request.get(), assign_response, &returned_response);
161 mir_wait_for(op_wh);
162
163 EXPECT_THAT(mir_platform_message_get_opcode(returned_response), Eq(opcode));

Subscribers

People subscribed via source and target branches