Review Request: Reimplemented the Peer Interface, removed some unused code and added the status bar.

[Daniele Elmo Domenichelli]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106037/#review17623
-----------------------------------------------------------


This is weird, it is now working even if I run both the accounts on the same computer, can you confirm that it is working for you as well?
I suspect that the problem was just with the org.freedesktop.DBus.Peer interface.

A few more small issues, but we are getting there...


kwhiteboard/kwhiteboard.cpp
<http://git.reviewboard.kde.org/r/106037/#comment13798>

    QLabel has no margin by default so the rightmost number is not very readable.
    Please add some margin.
    Perhaps you can also do "setFrameStyle(QFrame::Panel|QFrame::Sunken);" to make it look very professional (in that case you should probably set a fixed width)



kwhiteboard/kwhiteboard.cpp
<http://git.reviewboard.kde.org/r/106037/#comment13792>

    1) this is not translated
    2) I don't like it, just leave it empty... or use "Latency ???"



kwhiteboard/kwhiteboard.cpp
<http://git.reviewboard.kde.org/r/106037/#comment13797>

    I don't think that showing the whole error message in the status bar is a good idea here. Just show something short and similar to what you show at the beginning (perhaps something like "Latency ???")



kwhiteboard/kwhiteboard.cpp
<http://git.reviewboard.kde.org/r/106037/#comment13796>

    This is not translated. Also you should add ms at the end.
    You can do everything in just one line using something like:
    
    m_latencyLabel->setText(i18nc("%1 is the latency in milliseconds", "Latency: %1 ms", m_timer.elapsed()));



kwhiteboard/kwhiteboard.cpp
<http://git.reviewboard.kde.org/r/106037/#comment13793>

    1) m_event is for a member variable, this is not the case, rename it event like in the .h
    2) it gives an unused warning, you can add Q_UNUSED(event) before the if to suppress it


- Daniele Elmo Domenichelli


On Aug. 17, 2012, 6:05 p.m., Puneet Goyal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106037/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2012, 6:05 p.m.)
> 
> 
> Review request for Telepathy and Daniele Elmo Domenichelli.
> 
> 
> Description
> -------
> 
> Reimplemented the Peer Interface, removed some unused code and added the status bar.
> 
> 
> Diffs
> -----
> 
>   kwhiteboard/CMakeLists.txt 4a86efc 
>   kwhiteboard/kwhiteboard-handler.cpp ade1218 
>   kwhiteboard/kwhiteboard.h 33361c6 
>   kwhiteboard/kwhiteboard.cpp c16fd04 
>   kwhiteboard/main.cpp 4371285 
>   kwhiteboard/org.kde.DBus.Peer.xml PRE-CREATION 
>   kwhiteboard/peer.h PRE-CREATION 
>   kwhiteboard/peer.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106037/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Puneet Goyal
> 
>

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