|
»
[kopete-devel] Review Request: Make skype plugin use message states
|
View:
New views
8 Messages
—
Rating Filter:
Alert me
|
 |
[kopete-devel] Review Request: Make skype plugin use message states
by Bugzilla from btsai@vrwarp.com
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/ ----------------------------------------------------------- Review request for Kopete. Summary ------- Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. Diffs ----- /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 Diff: http://reviewboard.kde.org/r/1815/diff Testing ------- Chatted with friends with the patch applied. It appears to work. Thanks, Benson _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Pali
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/#review2595 ----------------------------------------------------------- /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1925> same as Skype::sendToChat /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1924> What do you return here? It return string "CHATMESSAGE <ID> STATUS SENDING". I think you need return only id (not full this string). For more see https://developer.skype.com/Docs/ApiDoc/CHATMESSAGE /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1926> Protocol version 4 is not supported. /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1923> For this use: d->connection % QString(....) instead d->connection.send(....) /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.cpp <http://reviewboard.kde.org/r/1815/#comment1928> Function, which return reply message from skype is operator% (not send). Dont modify this function. Instead d->connection.send(QString) use d->connection % QString in your code. This is what you need. /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.cpp <http://reviewboard.kde.org/r/1815/#comment1921> Please revert this back. Dont change code of skypeconnection. /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h <http://reviewboard.kde.org/r/1815/#comment1927> This function doesnt return any value. - Pali On 2009-10-09 05:44:33, Benson Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1815/ > ----------------------------------------------------------- > > (Updated 2009-10-09 05:44:33) > > > Review request for Kopete. > > > Summary > ------- > > Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 > > Diff: http://reviewboard.kde.org/r/1815/diff > > > Testing > ------- > > Chatted with friends with the patch applied. It appears to work. > > > Thanks, > > Benson > > _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Bugzilla from btsai@vrwarp.com
::
Rate this Message:
> On 2009-10-09 11:48:17, Pali Rohár wrote: > > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp, line 724 > > <http://reviewboard.kde.org/r/1815/diff/2/?file=12420#file12420line724> > > > > Protocol version 4 is not supported. Can the version 4 specific code be removed altogether then? - Benson ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/#review2595 ----------------------------------------------------------- On 2009-10-09 05:44:33, Benson Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1815/ > ----------------------------------------------------------- > > (Updated 2009-10-09 05:44:33) > > > Review request for Kopete. > > > Summary > ------- > > Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skypedbus/skypeconnection.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 > > Diff: http://reviewboard.kde.org/r/1815/diff > > > Testing > ------- > > Chatted with friends with the patch applied. It appears to work. > > > Thanks, > > Benson > > _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Bugzilla from btsai@vrwarp.com
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/ ----------------------------------------------------------- (Updated 2009-10-09 13:47:52.309736) Review request for Kopete. Changes ------- Addressing review. Summary ------- Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. Diffs (updated) ----- /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 Diff: http://reviewboard.kde.org/r/1815/diff Testing ------- Chatted with friends with the patch applied. It appears to work. Thanks, Benson _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Pali
::
Rate this Message:
> On 2009-10-09 11:48:17, Pali Rohár wrote: > > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp, line 724 > > <http://reviewboard.kde.org/r/1815/diff/2/?file=12420#file12420line724> > > > > Protocol version 4 is not supported. > > wrote: > Can the version 4 specific code be removed altogether then? Only version 8 is supported by kde4 version of skype plugin. Yes, you can delete it. - Pali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/#review2595 ----------------------------------------------------------- On 2009-10-09 13:47:52, Benson Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1815/ > ----------------------------------------------------------- > > (Updated 2009-10-09 13:47:52) > > > Review request for Kopete. > > > Summary > ------- > > Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 > > Diff: http://reviewboard.kde.org/r/1815/diff > > > Testing > ------- > > Chatted with friends with the patch applied. It appears to work. > > > Thanks, > > Benson > > _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Pali
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/#review2599 ----------------------------------------------------------- /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1934> return QString() instead "" /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp <http://reviewboard.kde.org/r/1815/#comment1935> return QString() instead "" /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h <http://reviewboard.kde.org/r/1815/#comment1933> Check if sentMessage returns somethink and if it is needed. - Pali On 2009-10-09 13:47:52, Benson Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1815/ > ----------------------------------------------------------- > > (Updated 2009-10-09 13:47:52) > > > Review request for Kopete. > > > Summary > ------- > > Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 > > Diff: http://reviewboard.kde.org/r/1815/diff > > > Testing > ------- > > Chatted with friends with the patch applied. It appears to work. > > > Thanks, > > Benson > > _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Bugzilla from btsai@vrwarp.com
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/ ----------------------------------------------------------- (Updated 2009-10-09 14:16:22.359909) Review request for Kopete. Changes ------- Address review. Summary ------- Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. Diffs (updated) ----- /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 Diff: http://reviewboard.kde.org/r/1815/diff Testing ------- Chatted with friends with the patch applied. It appears to work. Thanks, Benson _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
|
|
Re: [kopete-devel] Review Request: Make skype plugin use message states
by Pali
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1815/#review2803 ----------------------------------------------------------- I think, patch is OK now. Matt, it is possible to commit this? - Pali On 2009-10-09 14:16:22, Benson Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1815/ > ----------------------------------------------------------- > > (Updated 2009-10-09 14:16:22) > > > Review request for Kopete. > > > Summary > ------- > > Currently, the skype plugin does not show a message until it receives acknowledgement via dbus that the message is sent. This introduces a human noticeable between when the user presses enter and when the message shows up. The delay is even longer for the first message because skype has to setup the session. To improve the situation, I modified the skype plugin to use message status to indicate sending/sent and append the message instantly instead of waiting for the ack. > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/libskype/skype.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypeaccount.cpp 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.h 1030232 > /trunk/KDE/kdenetwork/kopete/protocols/skype/skypechatsession.cpp 1030232 > > Diff: http://reviewboard.kde.org/r/1815/diff > > > Testing > ------- > > Chatted with friends with the patch applied. It appears to work. > > > Thanks, > > Benson > > _______________________________________________ kopete-devel mailing list kopete-devel@... https://mail.kde.org/mailman/listinfo/kopete-devel |
| Free embeddable forum powered by Nabble | Forum Help |
