Mir

Code review comment for lp:~hikiko/mir/mir.dest-tmp

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

1. Not base classes, should not be virtual:
+ virtual ~SingleStepMatch() {}
+ virtual ~MockAndroidRegistrar() noexcept {}
+ virtual ~MockANativeWindow() {}
+ virtual ~MockImplementation() {}

2. This change is pointless, because -Wall already includes -Wnon-virtual-dtor, according to the docs...
68 -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -std=c++0x -Werror -Wall -fno-strict-aliasing -pedantic -Wextra -fPIC")
69 +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -std=c++0x -Werror -Wall -fno-strict-aliasing -pedantic -Wnon-virtual-dtor -Wextra -fPIC")

3. Is this a mistake?
81 -protected:

4. Pointless whitespace changes:
96 -
344 -
368 -

5. Pointless comment:
356 +//do not remove this empty destructor (unique_ptr)
Because you can't accidentally remove the empty virtual destructor without getting a linker error.

review: Needs Fixing

« Back to merge proposal