----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102301/#review5736 -----------------------------------------------------------
Overall, it's really good. A little bit of nit picking, and it will be perfect. nepomuk/kcm/statuswidget.cpp <http://git.reviewboard.kde.org/r/102301/#comment5086> Is this second m_service->isSuspended() call really required? nepomuk/kcm/statuswidget.cpp <http://git.reviewboard.kde.org/r/102301/#comment5087> Could the removal of the store size be done in another patch? nepomuk/kcm/statuswidget.cpp <http://git.reviewboard.kde.org/r/102301/#comment5092> I haven't tried this out, but looking at it I can see one obvious flaw - What if I pause the the indexer using some other means ( KCM, or dbus ), then the button's state will not change. I don't think it is a good idea to have a m_suspendedManually variable. It would be better to call suspend() or resume() and then wait till the status has changed and then update the button. You can hook a slot up to the statusChanged signal, or add custom signals for suspended and resumed nepomuk/kcm/statuswidget.ui <http://git.reviewboard.kde.org/r/102301/#comment5093> Maybe a slightly more descriptive name? - Vishesh On Aug. 12, 2011, 2:47 p.m., Smit Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102301/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2011, 2:47 p.m.) > > > Review request for Nepomuk. > > > Summary > ------- > > Now you can pause or resume strigi and see the currently indexed file. > > > Diffs > ----- > > nepomuk/controller/systray.cpp 6cf6bc9 > nepomuk/kcm/CMakeLists.txt 561d48a > nepomuk/kcm/nepomukserverkcm.cpp 74031b7 > nepomuk/kcm/statuswidget.h 0440745 > nepomuk/kcm/statuswidget.cpp 4798d7f > nepomuk/kcm/statuswidget.ui 359ec6e > > Diff: http://git.reviewboard.kde.org/r/102301/diff > > > Testing > ------- > > paused and resume couple of times and it works perfectly. > > > Screenshots > ----------- > > See > http://git.reviewboard.kde.org/r/102301/s/223/ > see again > http://git.reviewboard.kde.org/r/102301/s/224/ > with current status > http://git.reviewboard.kde.org/r/102301/s/226/ > > http://git.reviewboard.kde.org/r/102301/s/227/ > > > Thanks, > > Smit > >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
