[Kde-pim] Review Request 122647: Adopt Qt5 C++ idioms
Aleix Pol Gonzalez
aleixpol at kde.org
Wed Feb 25 14:59:13 GMT 2015
> On Feb. 25, 2015, 10:41 a.m., Vishesh Handa wrote:
> > akregator/src/akregator_part.cpp, line 445
> > <https://git.reviewboard.kde.org/r/122647/diff/2/?file=350531#file350531line445>
> >
> > I'm not too sure what the point of the QScopedPointer or std::auto_ptr is over here.
> >
> > The code is creating `LoadFeedListCommand` off the heap, and then assigning it to `m_loadFeedListCommand`.
> >
> > Maybe just use a simple pointer?
Yes, I changed it into a raw pointer and Kevin suggested to change back. I did because this way we preserve the spirit of the original code.
- Aleix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122647/#review76588
-----------------------------------------------------------
On Feb. 21, 2015, 3:06 p.m., Aleix Pol Gonzalez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122647/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2015, 3:06 p.m.)
>
>
> Review request for KDEPIM.
>
>
> Repository: kdepim
>
>
> Description
> -------
>
> Makes the code more easily readable by whomever is used to Qt code, especially the change from boost::bind().
>
> boost::shared_ptr -> QSharedPointer
> boost::weak_ptr -> QWeakPointer
> assert() -> Q_ASSERT()
> boost::bind() -> lambda
> std::auto -> QScopedPointer
>
>
> Diffs
> -----
>
> akregator/interfaces/article.h d13ae92
> akregator/src/CMakeLists.txt 98698f5
> akregator/src/loadfeedlistcommand.h f0e1b3f
> akregator/src/loadfeedlistcommand.cpp 45708a6
> akregator/src/mainwidget.h 98963fb
> akregator/src/mainwidget.cpp bbaf199
> akregator/src/progressmanager.h eb61757
> akregator/src/progressmanager.cpp af369f8
> akregator/src/searchbar.h bf59301
> akregator/src/searchbar.cpp e93f133
> akregator/src/selectioncontroller.h 1392375
> akregator/src/selectioncontroller.cpp 8fc7364
> akregator/src/subscriptionlistjobs.h 168b96d
> akregator/src/subscriptionlistjobs.cpp d391c8e
> akregator/src/subscriptionlistmodel.h 46fb817
> akregator/src/subscriptionlistmodel.cpp dfb17ea
> akregator/src/subscriptionlistview.cpp 02ac04d
> akregator/src/tabwidget.cpp 691be2f
> akregator/src/treenode.cpp 1d31542
> akregator/src/utils/filtercolumnsproxymodel.cpp 08d7593
> akregator/src/abstractselectioncontroller.h 20b241d
> akregator/src/akregator_part.h 90b2a0b
> akregator/src/akregator_part.cpp 021a105
> akregator/src/article.cpp c3bb8e4
> akregator/src/articleformatter.cpp 3c9d7ae
> akregator/src/articlejobs.h 28206d0
> akregator/src/articlejobs.cpp 0c4e6c4
> akregator/src/articlelistview.h 53b5c31
> akregator/src/articlelistview.cpp 94517f9
> akregator/src/articlemodel.h a1e5da3
> akregator/src/articlemodel.cpp 046e4ea
> akregator/src/articleviewer.h 334e92c
> akregator/src/articleviewer.cpp 81ee0a5
> akregator/src/browserframe.cpp a317046
> akregator/src/createfeedcommand.cpp 0aeaf62
> akregator/src/createfoldercommand.cpp c7a47b3
> akregator/src/deletesubscriptioncommand.h e92afc5
> akregator/src/deletesubscriptioncommand.cpp 0a63f34
> akregator/src/editsubscriptioncommand.h 5794c2b
> akregator/src/editsubscriptioncommand.cpp b23f8c7
> akregator/src/expireitemscommand.h 59ddd70
> akregator/src/expireitemscommand.cpp 718565a
> akregator/src/feed.cpp 56eb47d
> akregator/src/feediconmanager.cpp 722c130
> akregator/src/feedlist.h a61b750
> akregator/src/feedlist.cpp 916c5f6
> akregator/src/fetchqueue.cpp 79d0832
> akregator/src/folder.cpp f9769e4
> akregator/src/importfeedlistcommand.h 5a6ea78
> akregator/src/importfeedlistcommand.cpp 8406d82
> akregator/src/kernel.h 27e4240
> akregator/src/kernel.cpp 91236c5
> akregator/configuration/settings_advanced.cpp b36e7d7
> akregator/export/akregatorstorageexporter.cpp 8e32a4a
>
> Diff: https://git.reviewboard.kde.org/r/122647/diff/
>
>
> Testing
> -------
>
> Still builds, seems to work fine.
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
More information about the kde-pim
mailing list