Review Request 121656: Krita - Fix weak shared pointer copy constructor and assignment

Dmitry Kazakov dimula73 at gmail.com
Wed Dec 24 09:58:33 GMT 2014


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

Ship it!



krita/image/kis_shared_ptr.h
<https://git.reviewboard.kde.org/r/121656/#comment50522>

    Coding style



krita/image/kis_shared_ptr.h
<https://git.reviewboard.kde.org/r/121656/#comment50521>

    Coding style :(
    
    https://techbase.kde.org/Policies/Kdelibs_Coding_Style


Hi, Stefano!

Your patch is awesome! Especially removed doublechecks for logical operators (not sure how we got them here) and pesence of unittests. The only "trouble" is coding style of the braces in 'if' statements in the copy constructor. They should be present and they should be on the same line as the statement itself. You can just fix it and push the patch without another round of review.

- Dmitry Kazakov


On Dec. 24, 2014, 12:34 a.m., Stefano Bonicatti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121656/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2014, 12:34 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Now is possible to copy and assign an invalid weak pointer to a shared pointer or another weak pointer.
> Cleaned up a couple of redundant checks.
> Added some testcases to test the validity of the change.
> 
> 
> Diffs
> -----
> 
>   krita/image/kis_shared_ptr.h 85aee50 
>   krita/image/tests/kis_shared_ptr_test.h 9c9a35f 
>   krita/image/tests/kis_shared_ptr_test.cpp 6f44b1c 
> 
> Diff: https://git.reviewboard.kde.org/r/121656/diff/
> 
> 
> Testing
> -------
> 
> Compiles fine.
> Tests are all passing.
> 
> 
> Thanks,
> 
> Stefano Bonicatti
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141224/0dec0c31/attachment.htm>


More information about the calligra-devel mailing list