Review Request: Fix and cleanup of AdiumXtra styles installer
Dominik Schmidt
ich at dominik-schmidt.de
Wed Mar 21 15:50:22 UTC 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104355/#review11693
-----------------------------------------------------------
adiumxtra-protocol-handler/chat-style-installer.cpp
<http://git.reviewboard.kde.org/r/104355/#comment9282>
Can't this be done more elegantly? Fixing race conditions by just waiting a fixed time seems utterly wrong
adiumxtra-protocol-handler/main.cpp
<http://git.reviewboard.kde.org/r/104355/#comment9283>
You might wanna add yourself here ;-)
Regarding my own code... sorry, for hijacking your review request: should names be enclosed in ki18n or QLatin1String? And certainly the QLatin1String is missing for my email address, so if you add yourself maybe change that too ;-)
adiumxtra-protocol-handler/main.cpp
<http://git.reviewboard.kde.org/r/104355/#comment9285>
Why don't you invoke it directly but through the event loop?
lib/chat-window-style-manager.cpp
<http://git.reviewboard.kde.org/r/104355/#comment9284>
Aren't those required by the spec?
Are we providing defaults when they are not around?
Thanks for taking care, it's nice to see the code is not completely abandoned! :-)
- Dominik Schmidt
On March 21, 2012, 2:53 p.m., Daniele Elmo Domenichelli wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104355/
> -----------------------------------------------------------
>
> (Updated March 21, 2012, 2:53 p.m.)
>
>
> Review request for Telepathy, David Edmundson and Dominik Schmidt.
>
>
> Description
> -------
>
> AdimuXtra is currently broken, this set of patches fixes the installation of styles, I didn't try the installation of emoticons though
>
> Branch here: http://quickgit.kde.org/index.php?p=clones%2Fktp-text-ui%2Fddomenichelli%2Fktp-text-ui.git&a=shortlog&h=refs/heads/adiumxtra
>
>
> This addresses bug 282519.
> http://bugs.kde.org/show_bug.cgi?id=282519
>
>
> Diffs
> -----
>
> adiumxtra-protocol-handler/adiumxtra-protocol-handler.h 9b58210ad257d6ad8b802957cb7bcc66c0c5c2a9
> adiumxtra-protocol-handler/adiumxtra-protocol-handler.cpp 728a3722b9556c9c21196f8f8e8bad5c0f03dcb8
> adiumxtra-protocol-handler/chat-style-installer.cpp 1b2a1c95fc468d0c80007b057997ef4e91a0d306
> adiumxtra-protocol-handler/emoticon-set-installer.cpp e324df022300ec97dca9bbcb1193c5f5f89c8269
> adiumxtra-protocol-handler/main.cpp e1ed40abad717475800451236a5295e1520d3881
> lib/chat-window-style-manager.cpp 235fc5361f916ab0f1c1a05ec4e662232a917804
>
> Diff: http://git.reviewboard.kde.org/r/104355/diff/
>
>
> Testing
> -------
>
> Installing themes works!
>
>
> Thanks,
>
> Daniele Elmo Domenichelli
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120321/4f3b66b1/attachment.html>
More information about the KDE-Telepathy
mailing list