Mir

Merge lp://qastaging/~raof/mir/platform-probe-logging into lp://qastaging/mir

Proposed by Chris Halse Rogers
Status: Work in progress
Proposed branch: lp://qastaging/~raof/mir/platform-probe-logging
Merge into: lp://qastaging/mir
Prerequisite: lp://qastaging/~raof/mir/privatise-all-the-things
Diff against target: 1132 lines (+628/-28)
29 files modified
include/platform/mir/graphics/platform_probe_report.h (+45/-0)
include/platform/mir/options/configuration.h (+1/-0)
include/server/mir/default_server_configuration.h (+3/-0)
platform-ABI-sha1sums (+2/-1)
server-ABI-sha1sums (+3/-2)
src/platform/graphics/platform_probe.cpp (+7/-3)
src/platform/graphics/platform_probe.h (+2/-1)
src/platform/options/default_configuration.cpp (+20/-3)
src/platform/symbols.map (+1/-0)
src/server/graphics/default_configuration.cpp (+3/-3)
src/server/report/default_server_configuration.cpp (+9/-0)
src/server/report/logging/CMakeLists.txt (+1/-0)
src/server/report/logging/logging_report_factory.cpp (+6/-0)
src/server/report/logging/platform_probe_report.cpp (+55/-0)
src/server/report/logging/platform_probe_report.h (+55/-0)
src/server/report/logging_report_factory.h (+1/-0)
src/server/report/lttng/CMakeLists.txt (+1/-0)
src/server/report/lttng/lttng_report_factory.cpp (+6/-0)
src/server/report/lttng/platform_probe_report.cpp (+42/-0)
src/server/report/lttng/platform_probe_report.h (+47/-0)
src/server/report/lttng/platform_probe_report_tp.h (+57/-0)
src/server/report/lttng_report_factory.h (+1/-0)
src/server/report/null/CMakeLists.txt (+2/-1)
src/server/report/null/null_report_factory.cpp (+11/-0)
src/server/report/null/platform_probe_report.cpp (+32/-0)
src/server/report/null/platform_probe_report.h (+42/-0)
src/server/report/null_report_factory.h (+2/-0)
src/server/report/report_factory.h (+2/-0)
tests/unit-tests/graphics/test_platform_prober.cpp (+169/-14)
To merge this branch: bzr merge lp://qastaging/~raof/mir/platform-probe-logging
Reviewer Review Type Date Requested Status
Alan Griffiths Needs Fixing
Daniel van Vugt Abstain
Kevin DuBois (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Alexandros Frantzis (community) Approve
Review via email: mp+234064@code.qastaging.launchpad.net

Commit message

Add reporting for the graphics platform probe

Description of the change

Report (not log! ☺) the platform probing process

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Nits:

35 +public:
36 + PlatformProbeReport() = default;
37 + virtual ~PlatformProbeReport() = default;
38 +
39 + PlatformProbeReport(PlatformProbeReport const&) = delete;
40 + PlatformProbeReport& operator=(PlatformProbeReport const&) = delete;

For abstract base classes, we usually make the constructor and and deleted CopyAssign ops protected (not that it matters functionally).

371 +
452 +
530 +
531 +

Unnecessary blank lines.

review: Approve
1848. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

1849. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

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

looks okay to me. (Although I'm not sure what to do about the ABI numbers. Seems we are changing the binary compatibility of the server library by changing DefaultServerConfiguration though)

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

I think reporting and logging both have their place. Although I would have said this is a job for logging instead. But my opinion aside...

(1) The server ABI has been broken and needs bumping, due to: include/server/mir/default_server_configuration.h
Although you will likely run into the roadblock of bug 1293944 then. So we need at least one fix for bug 1293944 to land first.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I've now proposed the ABI bump as a separate branch, since this time round it's particularly troublesome.

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

24 conflicts encountered.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

I am inclined to agree with Daniel that this is really a case for logging. These aren't really interesting as instrumentation points and only as a historical record.

Unmerged revisions

1849. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

1848. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

1847. By Chris Halse Rogers

Remerge prereq

1846. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

1845. By Chris Halse Rogers

Ooops. Actually implement the PlatformProbeReport

1844. By Chris Halse Rogers

Log when server platforms fail to probe

1843. By Chris Halse Rogers

Merge prereq

1842. By Chris Halse Rogers

Merged private-library-loading into server-probe-logging.

1841. By Chris Halse Rogers

Trivial style cleanup

1840. By Chris Halse Rogers

Update platform and server SHA1sums

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches