----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124947/#review84451 -----------------------------------------------------------
ksmserver/screenlocker/greeter/greeterapp.cpp (line 140) <https://git.reviewboard.kde.org/r/124947/#comment58460> It might be nice to check if this pointer isn't 0 after the cast. Also, coding style. qobject_cast<KQuickAddons::QuickViewSharedEngine*> instead of qobject_cast<KQuickAddons::QuickViewSharedEngine *> Or the other way around, but then all the others need to change ;) Not quite sure which one is appropiate here, the style guideline isn't clear on this. I think the one with the space is the adviced way (which would mean that the rest needs to change, not this line) ksmserver/screenlocker/greeter/greeterapp.cpp (line 165) <https://git.reviewboard.kde.org/r/124947/#comment58461> Just a suggestion, not an issue. Can't this (and m_views where this one is inserted later on) be a smart pointer? That makes it go out of scope instead of relying on qDeleteAll(m_views) which is done in the destructor. Both methods work just fine, but i - personal preference - prefer smart pointers :) ksmserver/screenlocker/greeter/greeterapp.cpp (line 346) <https://git.reviewboard.kde.org/r/124947/#comment58462> coding style. QuickViewSharedEngine *view instead of QuickViewSharedEngine * view ksmserver/screenlocker/greeter/greeterapp.cpp (line 350) <https://git.reviewboard.kde.org/r/124947/#comment58463> idem I can't really give a +1.. I just don't know this or the implications it might have. So just a style review then :) - Mark Gaiser On aug 27, 2015, 8:34 a.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124947/ > ----------------------------------------------------------- > > (Updated aug 27, 2015, 8:34 a.m.) > > > Review request for Plasma and David Edmundson. > > > Repository: plasma-workspace > > > Description > ------- > > In a multi-screen setup we have multiple views showing the same Qml > scene. Let's share the engine for all views. > > > Diffs > ----- > > ksmserver/screenlocker/greeter/CMakeLists.txt > 4fb679f838a80f11eb8e4b4f5cb5ef04dc39c0f7 > ksmserver/screenlocker/greeter/greeterapp.h > ed278e492a9a237f65f4c1600360f84fb25bdad7 > ksmserver/screenlocker/greeter/greeterapp.cpp > b500ba44c2b483d7372ca46840152c90ef5f798c > > Diff: https://git.reviewboard.kde.org/r/124947/diff/ > > > Testing > ------- > > This makes creating a second view basically free (from timing perspective) > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
