Review Request 118984: KoPart argument of KoDocument and descendants should not be optional

[Jarosław Staniek]

> On June 29, 2014, 4:45 a.m., Inge Wallin wrote:
> > sheets/part/Doc.cpp, line 153
> > <https://git.reviewboard.kde.org/r/118984/diff/1/?file=285017#file285017line153>
> >
> >     Not strictly necessary since DocBase::DocBase already has the same Q_ASSERT. If you want to keep it, I'm fine with that too.

Sure it has but having the assert here indicates that it's not wrong use of DocBase that hurts but Doc 'knows' the assertion should happen. By principle it's just like adding assertion or check for invalid index somewhere despite having it in QList::at() already.


- Jarosław


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


On June 29, 2014, 9:14 p.m., Jarosław Staniek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118984/
> -----------------------------------------------------------
> 
> (Updated June 29, 2014, 9:14 p.m.)
> 
> 
> Review request for Calligra and Boudewijn Rempt.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> KoPart argument of KoDocument and descendants should not be optional
> 
> Currently KoPart is required, there is even assertion for, so express this in the API.
> Current API apparently caused one crash (in spreadsheet plugin of Kexi).
> 
> Also put 2 more assertions as early as possible.
> 
> Finally, fixed outdated docs.
> 
> 
> Diffs
> -----
> 
>   karbon/ui/KarbonDocument.cpp eed34ea 
>   krita/ui/kis_doc2.h 657fabc 
>   krita/ui/kis_doc2.cc aad9ae5 
>   plan/kptmaindocument.h 5c7e44d 
>   plan/kptmaindocument.cpp 9b9f485 
>   sheets/DocBase.h 946e9cd 
>   sheets/DocBase.cpp b01a200 
>   sheets/part/Doc.h bf8341c 
>   sheets/part/Doc.cpp 44fd7ab 
>   words/part/KWDocument.cpp 533b8e1 
> 
> Diff: https://git.reviewboard.kde.org/r/118984/diff/
> 
> 
> Testing
> -------
> 
> Builds, nobody is used the argument-less constructor (after https://git.reviewboard.kde.org/r/118983/ is pushed)
> 
> 
> Thanks,
> 
> Jarosław Staniek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140629/466228f5/attachment.htm>