[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-telepathy
Subject: Re: Review Request 110334: %time Variable to get the current UTC time in the away message
From: "Daniele E. Domenichelli" <daniele.domenichelli () gmail ! com>
Date: 2013-05-21 14:35:41
Message-ID: 20130521143541.22229.77818 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110334/#review32898
-----------------------------------------------------------
The code looks fine, but I'd rather use the actual timezone of the user that went \
away instead of the UTC (see https://qt-project.org/doc/qt-4.8/qtime.html#toString \
"t" option). But if the others are ok with it, it's ok with me as well.
P.S. Sorry if it took so long for my review but I'm very busy in this period :(
- Daniele E. Domenichelli
On May 9, 2013, 3:19 p.m., Lucas Betschart wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110334/
> -----------------------------------------------------------
>
> (Updated May 9, 2013, 3:19 p.m.)
>
>
> Review request for Telepathy and Daniele E. Domenichelli.
>
>
> Description
> -------
>
> I've added an %time option which will get replaced by the current UTC time.
>
> To make this option visible I've changed the tool tip (I think the first thing a \
> user would do if he wants no away message is to remove the message text [if there \
> is one, which isn't by default], so this double information can be replaced).
> Adding an extra widget for drag n drop like in now playing would be too much for \
> this I think. It would blow up the UI if I add this under every lineedit (as more \
> stuff is visible on the first sight for the user as more complicated it looks) and \
> it would be confusing if I add it only once in a central place, so I decided to \
> just inform the user through a tool tip (it's also not a important feature, so it's \
> ok if it's a little bit hidden).
>
> This addresses bug https://bugs.kde.org/show_bug.cgi?id=300849.
> http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=300849
>
>
> Diffs
> -----
>
> autoaway.cpp 06cb8e05d84f250e59a373461cb709ec5d400471
> config/telepathy-kded-config.cpp 780a882858c1fed4968b71cf4c5832f9d43f7b7a
> screensaveraway.cpp c7fdd75aa2217a44ec849dc9acfbf20020524bdb
>
> Diff: http://git.reviewboard.kde.org/r/110334/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lucas Betschart
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/110334/">http://git.reviewboard.kde.org/r/110334/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The code looks fine, but \
I'd rather use the actual timezone of the user that went away instead of the UTC \
(see https://qt-project.org/doc/qt-4.8/qtime.html#toString "t" option). But \
if the others are ok with it, it's ok with me as well.
P.S. Sorry if it took so long for my review but I'm very busy in this period \
:(</pre> <br />
<p>- Daniele E.</p>
<br />
<p>On May 9th, 2013, 3:19 p.m. UTC, Lucas Betschart wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Telepathy and Daniele E. Domenichelli.</div>
<div>By Lucas Betschart.</div>
<p style="color: grey;"><i>Updated May 9, 2013, 3:19 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I've added an %time option which will get replaced by the current \
UTC time.
To make this option visible I've changed the tool tip (I think the first thing a \
user would do if he wants no away message is to remove the message text [if there is \
one, which isn't by default], so this double information can be replaced).
Adding an extra widget for drag n drop like in now playing would be too much for this \
I think. It would blow up the UI if I add this under every lineedit (as more stuff is \
visible on the first sight for the user as more complicated it looks) and it would be \
confusing if I add it only once in a central place, so I decided to just inform the \
user through a tool tip (it's also not a important feature, so it's ok if \
it's a little bit hidden).</pre> </td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=300849">https://bugs.kde.org/show_bug.cgi?id=300849</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autoaway.cpp <span style="color: \
grey">(06cb8e05d84f250e59a373461cb709ec5d400471)</span></li>
<li>config/telepathy-kded-config.cpp <span style="color: \
grey">(780a882858c1fed4968b71cf4c5832f9d43f7b7a)</span></li>
<li>screensaveraway.cpp <span style="color: \
grey">(c7fdd75aa2217a44ec849dc9acfbf20020524bdb)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110334/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic