> On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > Very nice, thanks a lot for this! Couple comments below :)
Thanks for the fast review. > On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > config/telepathy-kded-config.cpp, line 91 > > <http://git.reviewboard.kde.org/r/110260/diff/3/?file=141623#file141623line91> > > > > I wonder if we should cache this i18n string so the translators don't > > have to translate it 3x (not your fault, I'll probably do it in a separate > > patch). They don't have to. Or at least I think they don't have to, because the new entry already appears translated in my UI (it's default German). So no changes needed. > On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > config/telepathy-kded-config.ui, line 10 > > <http://git.reviewboard.kde.org/r/110260/diff/3/?file=141624#file141624line10> > > > > Is this needed? Set back. Was made by Qt Designer. > On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > lockingscreenaway.h, line 3 > > <http://git.reviewboard.kde.org/r/110260/diff/3/?file=141625#file141625line3> > > > > I didn't write this :) Add yourself here (you can leave me as the > > original author, but basically it's your code) Oh, forgot the header^^ > On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > lockingscreenaway.cpp, line 52 > > <http://git.reviewboard.kde.org/r/110260/diff/3/?file=141626#file141626line52> > > > > I think more proper name would be screen-lock-away, probably same for > > the class name ("Locking screen" sounds weird) Changed to screen-saver-away (also file & classname) since that's more accurate (that's the trigger). > On May 1, 2013, 9:08 p.m., Martin Klapetek wrote: > > config/telepathy-kded-config.ui, line 348 > > <http://git.reviewboard.kde.org/r/110260/diff/3/?file=141624#file141624line348> > > > > I don't think we need another groupbox, especially for a single option; > > put it under the "Auto away" groupbox > > > > I'd suggest to put another horizontal line below the "Not available" in > > autoaway and put single checkbox there "[] Set my status to away when the > > screen is locked" and the input box below Changed - Lucas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110260/#review31853 ----------------------------------------------------------- On May 1, 2013, 8:37 p.m., Lucas Betschart wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110260/ > ----------------------------------------------------------- > > (Updated May 1, 2013, 8:37 p.m.) > > > Review request for Telepathy. > > > Description > ------- > > Plugin to set status "Away" when screen gets locked. > Forked from autoaway.h/cpp. > > > This addresses bug https://bugs.kde.org/show_bug.cgi?id=290998. > > http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290998 > > > Diffs > ----- > > CMakeLists.txt a339495773875954b668875a28c074f7ac7f733f > config/telepathy-kded-config.h b1bf626e0fbb9fc0705d95c9d190be5023ace876 > config/telepathy-kded-config.cpp 50c176d583aafdeedcf70442d58336afb5db3c19 > config/telepathy-kded-config.ui 54ebc542d7d04c14c7122d2ca3c2e0533b2c3b21 > lockingscreenaway.h PRE-CREATION > lockingscreenaway.cpp PRE-CREATION > telepathy-module.h 748437d6268fce49ef276ab996d18cbcb93da025 > telepathy-module.cpp d3f223a8391e232c74a5a03b9277f6c518c4d2ed > > Diff: http://git.reviewboard.kde.org/r/110260/diff/ > > > Testing > ------- > > Manually by myself. > > > Thanks, > > Lucas Betschart > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
