Mir

Merge lp:~vanvugt/mir/no-common-log into lp:mir

  1. no-common-log
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 2312
Proposed branch: lp:~vanvugt/mir/no-common-log
Merge into: lp:mir
Diff against target: 12 lines (+1/-1)
1 file modified
src/client/default_connection_configuration.cpp (+1/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/no-common-log
Reviewer Review Type Date Requested Status
Chris Halse Rogers Abstain
Alan Griffiths Needs Fixing
Kevin DuBois (community) Approve
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+248764@code.launchpad.net

Commit message

Turn off default logging reports that might spam a client's output stream (LP: #1414883). If someone wants the report, they can turn it on.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2303
http://jenkins.qa.ubuntu.com/job/mir-ci/2920/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1225
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1224
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1182/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/917
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/917/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1182
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1182/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4225/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17807

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2304
http://jenkins.qa.ubuntu.com/job/mir-ci/2926/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1233
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1232
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1190
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/923
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/923/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1190
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1190/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4232
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17836

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

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

A quick check indicates that mir::load_library from src/platform/shared_library_loader.cpp is not used in the Mir code (so removing the mir::log_info call there has no effect currently).

Are any downstreams using the mir::load_library function? If not, we should remove it.

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

> A quick check indicates that mir::load_library from
> src/platform/shared_library_loader.cpp is not used in the Mir code (so
> removing the mir::log_info call there has no effect currently).
>
> Are any downstreams using the mir::load_library function? If not, we should
> remove it.

Definitely not - we don't even compile the code.

Revision history for this message
Kevin DuBois (kdub) :
review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Has conflicts (as src/platform/shared_library_loader.cpp has been removed from trunk).

Otherwise OK

review: Needs Fixing
Revision history for this message
Chris Halse Rogers (raof) wrote :

I'm not unhappy with this, mainly because I intend to remove SharedLibraryProberReport now that we've got reasonable logging support in the client library.

I still disagree that LP: #1414883 is a bug :)

review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/client/default_connection_configuration.cpp'
2--- src/client/default_connection_configuration.cpp 2015-02-04 08:45:38 +0000
3+++ src/client/default_connection_configuration.cpp 2015-02-11 07:06:24 +0000
4@@ -226,7 +226,7 @@
5 [this] () -> std::shared_ptr<mir::SharedLibraryProberReport>
6 {
7 auto val_raw = getenv("MIR_CLIENT_SHARED_LIBRARY_PROBER_REPORT");
8- std::string const val{val_raw ? val_raw : log_opt_val};
9+ std::string const val{val_raw ? val_raw : off_opt_val};
10 if (val == log_opt_val)
11 return std::make_shared<mir::logging::SharedLibraryProberReport>(the_logger());
12 else if (val == lttng_opt_val)

Subscribers

People subscribed via source and target branches