Review Request 110260: Plugin to set status "Away" when screen gets locked.

[Lucas Betschart]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130501/96c1b2b4/attachment-0001.html>