Review Request: Activities KCM

[Ivan Čukić]

> On Aug. 22, 2012, 10:33 p.m., David Edmundson wrote:
> > src/workspace/settings/CMakeLists.txt, line 15
> > <http://git.reviewboard.kde.org/r/106130/diff/1/?file=80118#file80118line15>
> >
> >     capital letter in class filename isn't typically usual, and doesn't match any other cpp in kactivities repo.

The library is lower-case, service is camel-case

For the other parts of the repo, there is no rule, though I intend to make them all (apart from the library) camel-case.


> On Aug. 22, 2012, 10:33 p.m., David Edmundson wrote:
> > src/workspace/settings/BlacklistedApplicationsModel.cpp, line 113
> > <http://git.reviewboard.kde.org/r/106130/diff/1/?file=80117#file80117line113>
> >
> >     const &

val is intentionally introduced to serve as a short-hand for 'const auto' to get developers to declare variables as const if possible. (Like most, I don't have the culture to declare things as const as often as it is possible)

see README


> On Aug. 22, 2012, 10:33 p.m., David Edmundson wrote:
> > src/workspace/settings/BlacklistedApplicationsModel.cpp, line 92
> > <http://git.reviewboard.kde.org/r/106130/diff/1/?file=80117#file80117line92>
> >
> >     if applications.length is 0, you remove 0 to -1, and that gives you a crash (well qFatal)
> >     
> >

replaced with beginRemoveRows(QModelIndex(), 0, qMax(0, d->applications.length() - 1));


- Ivan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106130/#review17886
-----------------------------------------------------------


On Aug. 22, 2012, 10:04 p.m., Ivan Čukić wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106130/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2012, 10:04 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> System settings module for activities
> 
> 
> Diffs
> -----
> 
>   cmake/modules/c++11-test-initializer-lists-N2672.cpp PRE-CREATION 
>   src/CMakeLists.txt 63ed737 
>   src/config-features.h.cmake bde6b66 
>   src/service/Application.cpp 5dadbca 
>   src/service/plugins/sqlite/StatsPlugin.cpp 6f24be8 
>   src/service/plugins/virtualdesktopswitch/virtualdesktopswitch.cpp 568e0f9 
>   src/workspace/CMakeLists.txt 8a6e1bb 
>   src/workspace/settings/BlacklistedApplicationsModel.h PRE-CREATION 
>   src/workspace/settings/BlacklistedApplicationsModel.cpp PRE-CREATION 
>   src/workspace/settings/CMakeLists.txt PRE-CREATION 
>   src/workspace/settings/MainConfigurationWidget.h PRE-CREATION 
>   src/workspace/settings/MainConfigurationWidget.cpp PRE-CREATION 
>   src/workspace/settings/kcm_activities.cpp PRE-CREATION 
>   src/workspace/settings/kcm_activities.desktop PRE-CREATION 
>   src/workspace/settings/qml/BlacklistApplicationView.qml PRE-CREATION 
>   src/workspace/settings/ui/MainConfigurationWidgetBase.ui PRE-CREATION 
>   src/workspace/settings/ui/kdeclarativeview.h PRE-CREATION 
>   src/workspace/settings/ui/kdeclarativeview.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan Čukić
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120822/332b9c71/attachment.html>