Review Request 125951: [calendar] Move the plugins handling to a separate class

[Martin Klapetek]

> On Nov. 5, 2015, 1:41 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/calendarplugin.cpp, line 33
> > <https://git.reviewboard.kde.org/r/125951/diff/1/?file=414996#file414996line33>
> >
> >     leaks
> >     
> >     why are you setting cpp ownership anyway?

Yeah so I was wondering what should it be parented to? The engine is now shared right? So that wouldn't delete it with the applet being unloaded. The scriptEngine?

As for cpp ownership, to make sure qml never randomly deletes it, otherwise it would cause reloading of all the plugins.


> On Nov. 5, 2015, 1:41 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 271
> > <https://git.reviewboard.kde.org/r/125951/diff/1/?file=414998#file414998line271>
> >
> >     what if this is called before setSource?

It shouldn't because setSourceData is called in Calendar constructor (in qml it's the CalendarBackend component) and the setPluginsManager is called in Component.onCompleted of CalendarBackend, so this /should/ be called last.

But even if not, it shouldn't be a problem cause the events that would arrive in the connected signals would be stored in m_eventsData, then when setSourceData is called, it calls reset() which should reset all attached views and so the events would get to the view through refetching all data().


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125951/#review88031
-----------------------------------------------------------


On Nov. 4, 2015, 8:24 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125951/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 8:24 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> This is also made a QML singleton that will be used for the applet
> config view where it will add the plugin configs once we add that
> possibility.
> 
> The same instance is then set to the DaysModel from QML.
> 
> (this depends on https://git.reviewboard.kde.org/r/125817/ which awaits ship it)
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/calendar/CMakeLists.txt 40ead911ad5208cae5dbe5333d227f9f8a0d9154 
>   src/declarativeimports/calendar/calendarplugin.cpp bafe80cf7520a08312abfd1dbd6d4648a6710175 
>   src/declarativeimports/calendar/daysmodel.h a5bdac98627f7efa76bd4afd239469b53e06690b 
>   src/declarativeimports/calendar/daysmodel.cpp 2d059a8e8636565adbe52811e602fff37a5eb157 
>   src/declarativeimports/calendar/eventpluginsmanager.h PRE-CREATION 
>   src/declarativeimports/calendar/eventpluginsmanager.cpp PRE-CREATION 
>   src/declarativeimports/calendar/qml/MonthView.qml f698934f850ef3a917b9611c9f9a40c369b23f6c 
> 
> Diff: https://git.reviewboard.kde.org/r/125951/diff/
> 
> 
> Testing
> -------
> 
> Calendar events are still correctly displayed
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151105/f27f1f6e/attachment.html>