Code review comment for lp:~dandrader/qtmir/multiSurfaceApp

Revision history for this message
Gerry Boland (gerboland) wrote :

Please merge trunk, many conflicts have appeared.

+void Session::appendSurface
+void Session::reallyAppendSurface
these names confuse me. Perhaps rename to something like:
appendSurface -> registerSurface
reallyAppendSurface -> appendSurface

 qDebug() << "Application::suspend - no surface to call stopFrameDropper() on!";
I think we can move this to category logging

+ return const_cast<ObjectListModel<MirSurfaceInterface>*>(&m_surfaces);
const_cast needed here? Can't the pointer be a pointer to a const object?

=== modified file 'src/modules/Unity/Application/session.h'
+ QList<MirSurfaceInterface*> m_blankSurfaces;
What use is this? I only see you append to and remove from this list, but never use it for anything.

+ Q_PROPERTY(MirSurfaceInterface* lastSurface READ lastSurface NOTIFY lastSurfaceChanged CONSTANT);
Can you add a "REMOVEME" comment to this, this is not an API I want to keep. It also not very correct, as what if the surface pointed to is removed, but other surfaces exist - should it not be updated?

review: Needs Fixing

« Back to merge proposal