> 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ć > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
