Mir

Code review comment for lp:~raof/mir/probe-client-drivers

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

(2) Unnecessary (and undesirable) formatting changes:
575 -target_link_libraries(mirclient
576 +target_link_libraries(
577 + mirclient
578 +
because the first parameter is special and independent of those that follow it.

(4) Minor optimization: You can avoid a template instantiation using std::atomic_bool for
1250 + std::atomic<bool> connect_done;

(8) The library re-ordering appears to be pointless, or is there a reason?...
benchmarks/frame-uniformity/CMakeLists.txt
Can we revert all changes to that file?

(9) Clients now start, but the wrapper that makes it possible feels inelegant...
$ ls -a bin/
mir_acceptance_tests
.mir_acceptance_tests-uninstalled
mir_demo_client_basic
.mir_demo_client_basic-uninstalled
mir_demo_client_cursors
.mir_demo_client_cursors-uninstalled
mir_demo_client_display_config
.mir_demo_client_display_config-uninstalled
mir_demo_client_eglcounter
.mir_demo_client_eglcounter-uninstalled
...
I suspect we can do better without needing a wrapper. But if we do use this then please don't hide binaries from developers: ".foo-uninstalled" should be "foo.bin"

(10) wrapper.c compiles to 10KB, but could be replaced with a trivial shell script instead.

(11) This is new on the end. What's it do?
1343 +} MIR_COMMON_3;

review: Needs Fixing

« Back to merge proposal