Mir

Merge lp:~alan-griffiths/mir/fix-1391923 into lp:mir

  1. fix-1391923
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 2056
Proposed branch: lp:~alan-griffiths/mir/fix-1391923
Merge into: lp:mir
Diff against target: 50 lines (+17/-1)
3 files modified
include/server/mir/server.h (+8/-0)
server-ABI-sha1sums (+1/-1)
src/server/server.cpp (+8/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1391923
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Daniel van Vugt Needs Information
Kevin DuBois (community) Approve
Review via email: mp+241585@code.launchpad.net

Commit message

server: fix Server::add_configuration_option() overloads to treat char const* like std::string (and not like bool)

Description of the change

server: fix Server::add_configuration_option() overloads to treat char const* like std::string (and not like bool)

To post a comment you must log in.
Revision history for this message
Kevin DuBois (kdub) wrote :

looks good to me

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

PASSED: Continuous integration, rev:2051
http://jenkins.qa.ubuntu.com/job/mir-ci/2085/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/96
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/96
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/76
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/74
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/74/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/76
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/76/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3368
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15772

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

If the parameter in question is read-only, why is it a std::string?

Are all use cases more like (const char *)? If so then why not use that as the primary interface?

review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> If the parameter in question is read-only, why is it a std::string?
>
> Are all use cases more like (const char *)? If so then why not use that as the
> primary interface?

The cost of creating a std::string is low (SSO is likely), happens once during initialization and is amortized over the whole execution time.

FWIW (it's an implementation detail) a string is created anyway as boost.Options holds the default as a string.

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

PASSED: Continuous integration, rev:2053
http://jenkins.qa.ubuntu.com/job/mir-ci/2092/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/87
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/81
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/81/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/87
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/87/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3377
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15790

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

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

OK.

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

PASSED: Continuous integration, rev:2054
http://jenkins.qa.ubuntu.com/job/mir-ci/2094/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/111
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/111
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/83
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/83/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/91
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/91/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3380
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15800

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/server.h'
2--- include/server/mir/server.h 2014-11-12 15:32:55 +0000
3+++ include/server/mir/server.h 2014-11-13 14:21:10 +0000
4@@ -107,6 +107,14 @@
5 void add_configuration_option(
6 std::string const& option,
7 std::string const& description,
8+ char const* default_value);
9+
10+ /// Add user configuration option(s) to Mir's option handling.
11+ /// These will be resolved during initialisation from the command line,
12+ /// environment variables, a config file or the supplied default.
13+ void add_configuration_option(
14+ std::string const& option,
15+ std::string const& description,
16 bool default_value);
17
18 /// Add user configuration option(s) to Mir's option handling.
19
20=== modified file 'server-ABI-sha1sums'
21--- server-ABI-sha1sums 2014-11-12 15:32:55 +0000
22+++ server-ABI-sha1sums 2014-11-13 14:21:10 +0000
23@@ -99,7 +99,7 @@
24 993e9f458ffc4288d304413f3fa0b1dcc95a093d include/server/mir/scene/surface_observer.h
25 7ef3e99901168cda296d74d05a979f47bf9c3ff1 include/server/mir/server_action_queue.h
26 8d83a51c278b8b71866d2178d9b6387c1f91a7d0 include/server/mir/server_configuration.h
27-ec6f00e14a5c39ad6808a7e4c63ff7b84cfc3cf4 include/server/mir/server.h
28+f8083e0d3097da12426571ef72ff574926aae138 include/server/mir/server.h
29 86098b500339bfccd07a9bed8298f75a68b18f5c include/server/mir/server_status_listener.h
30 860c04f32b60e680140148dc9dc2295de145b9c1 include/server/mir/shell/display_layout.h
31 6a2107b01feae13060d5c305804906e53c52e0be include/server/mir/shell/focus_controller.h
32
33=== modified file 'src/server/server.cpp'
34--- src/server/server.cpp 2014-11-12 15:32:55 +0000
35+++ src/server/server.cpp 2014-11-13 14:21:10 +0000
36@@ -489,6 +489,14 @@
37 void mir::Server::add_configuration_option(
38 std::string const& option,
39 std::string const& description,
40+ char const* default_value)
41+{
42+ add_configuration_option(option, description, std::string{default_value});
43+}
44+
45+void mir::Server::add_configuration_option(
46+ std::string const& option,
47+ std::string const& description,
48 bool default_)
49 {
50 verify_setting_allowed(self->server_config);

Subscribers

People subscribed via source and target branches