Code review comment for lp:~gerboland/qtmir/qmirserver-hides-mirserver

> In src/platforms/mirserver/qmirserver.h
>
> """
> bool run();
> Q_SLOT bool stop();
> """
>
> Again, thinking about having it as public API: you should probably document
> these methods, answering questions like: Is it a blocking call or does it
> return immediately?
>
> What's the user expected to do with those return values? I don't see you using
> them for anything.

I was planning to document things later. But I can start now if you'd prefer.

> """
> connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit,
> d->serverWorker, &MirServerWorker::stop, Qt::DirectConnection); //
> see FIXME in MirServerWorker
> """
>
> """
> if (!d->serverWorker->waitForMirStartup())
> {
> qCritical() << "ERROR: QMirServer - Mir failed to start";
> QCoreApplication::quit();
> """
>
> Isn't QMirServer going out of its territory when it modifies QCoreApplication
> directly like that?
> I would expect the user of QMirServer to do this instead, like:
>
> """
> if (!qmirServer->run()) {
> QCoreApplication::quit();
> }
> """

You're totally right, this is also the direction I wanted to go. I was hesitant to do so, as I feared it along with all the other changes would make quite a large MR. So your suggestion is to be the next step: have the QMirServer user listen for mir or qt shutdown, and react accordingly. Should I just do that now?