Review Request: Adds d-pointers to Kexi classes in kexi/core
Jarosław Staniek
staniek at kde.org
Sun Dec 2 23:15:25 GMT 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107515/#review22917
-----------------------------------------------------------
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17498>
.. "by calling setFocusableWidget(bool) in your class' constructor."
(and please add this method (in protected area))
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17497>
by the way, please also move this implementation to .cpp
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17496>
arentDataItemInterface()->signalValueChanged()
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17502>
by the way please change to parentDataItemInterface() to match the name of setter
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17494>
-> "value (see originalValue())."
kexi/core/kexidataiteminterface.h
<http://git.reviewboard.kde.org/r/107515/#comment17495>
-> "it should be set to current originalValue() + \a add, if possible."
kexi/core/kexidataiteminterface.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17499>
Convention: Please use initialization list instead like we originally did.
kexi/core/kexidataiteminterface.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17500>
Please be more careful, this introduces a bug - should be true.
kexi/core/kexiinternalpart.h
<http://git.reviewboard.kde.org/r/107515/#comment17503>
not needed here
kexi/core/kexiinternalpart.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17504>
destructors shall be put right after constructors
kexi/core/kexiinternalpart.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17505>
please use initializers list like we did originally
kexi/core/kexiinternalpart.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17506>
use part->uniqueWidget() not part->d->...
kexi/core/kexiinternalpart.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17507>
use part->setUniqueWidget()
kexi/core/kexiinternalpart.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17508>
like above, don't use part->d->...
kexi/core/kexipartmanager.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17509>
use initializer list
kexi/core/kexipartmanager.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17510>
lines 71..73 not needed because initialization happens in Private()
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17511>
use initialization list
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17512>
missing d(new Private(driverName, databaseName))
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17513>
missing d(new Private(driverName, QString()))
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17515>
d->
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17516>
d->
kexi/core/kexiprojectconnectiondata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17514>
please move right after constructors
kexi/core/kexistartupdata.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17517>
please use initialization list as before
kexi/core/kexitextmsghandler.cpp
<http://git.reviewboard.kde.org/r/107515/#comment17518>
use initialization list as before
- Jarosław Staniek
On Dec. 2, 2012, 4:49 p.m., Andrey Inishev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107515/
> -----------------------------------------------------------
>
> (Updated Dec. 2, 2012, 4:49 p.m.)
>
>
> Review request for Calligra and Jarosław Staniek.
>
>
> Description
> -------
>
> Patch for kexi/core, http://www.google-melange.com/gci/task/view/google/gci2012/8009216
>
>
> Diffs
> -----
>
> kexi/core/kexidataiteminterface.h 4258451
> kexi/core/kexidataiteminterface.cpp a5ad759
> kexi/core/kexiinternalpart.h 08178ed
> kexi/core/kexiinternalpart.cpp 253e77d
> kexi/core/kexipartmanager.h 9cb2356
> kexi/core/kexipartmanager.cpp cf17f3a
> kexi/core/kexiprojectconnectiondata.h b88a1f9
> kexi/core/kexiprojectconnectiondata.cpp 0df2793
> kexi/core/kexistartupdata.h 2147465
> kexi/core/kexistartupdata.cpp 45fdd71
> kexi/core/kexitextmsghandler.h 29adc64
> kexi/core/kexitextmsghandler.cpp 71011c9
>
> Diff: http://git.reviewboard.kde.org/r/107515/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrey Inishev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20121202/987cf09b/attachment.htm>
More information about the calligra-devel
mailing list