> On Dec. 16, 2014, 7:51 p.m., Boudewijn Rempt wrote:
> > Maybe we can move the check for whether the canvas is the same to the 
> > KoCanvasObserver code?

setCanvas is called from another place too, i put the check there so that one 
doesn't have to remember to make the same check everytime one calls it. Since 
multiple developers works on this and maybe new calls could appear, i preferred 
to stay on the safe side, otherwise maybe, if the check is moved, it should be 
written in the documentation that it doesn't make the check.
Right now setCanvas is harmless if called 2 times with the same canvas but who 
knows in the future :P.


- Stefano


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


On Dec. 16, 2014, 4:26 p.m., Stefano Bonicatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121559/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2014, 4:26 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> OverviewDockerDock: no need to call the code in setCanvas if the canvas to be 
> set is the same.
> 
> KisZoomManager: the zoom slider widget is created with no parent and then 
> added to a QStatusBar, when it is then removed from it (when an image is 
> closed), it is not deleted because QStatusBar::removeWidget doesn't delete 
> the widget (as Qt Doc says). 
> Moreover each time an image is closed KisZoomManager is deleted and then 
> recreated with a new slider when an image is opened. The patch fixes the leak.
> 
> 
> Diffs
> -----
> 
>   krita/plugins/extensions/dockers/overview/overviewdocker_dock.cpp 68bd6fe 
>   krita/ui/kis_zoom_manager.cc 50f0e98 
> 
> Diff: https://git.reviewboard.kde.org/r/121559/diff/
> 
> 
> Testing
> -------
> 
> Compiles against calligra/2.9.
> Doesn't leak anymore.
> 
> 
> Thanks,
> 
> Stefano Bonicatti
> 
>

_______________________________________________
calligra-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to