----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/6339/#review9677 -----------------------------------------------------------
Most changes needed are cosmetic or related to variable names/coding style. I don't use the Yahoo protocol, so I can't test your changes, so I'm assuming they work fine. As for bug 194833, you could try to ask for more information on the bug report to see if these changes fix it. >From a code management point of view, I'd rather see these changes in >different patches (the ones to the receiving end don't seem to be related to >the sending end), but ReviewBoard isn't very helpful for this :/ trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.h <http://svn.reviewboard.kde.org/r/6339/#comment10807> Coding style: foo( bar ) instead of foo(bar) trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10808> Can you add a TODO: here to make it easier to spot this comment? trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10809> If the check is disabled, I'd just comment it out or put it inside an #if 0 block. trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10810> Coding style: set to 0, not NULL (also applies to m_transferJob). trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10811> Can you change these #define's to const int's? trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10812> const int; can you also change the name to something more meaningful? trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10813> const int; can you use a more meaningful variable name? trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10814> Same thing about const and variable names :) trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10815> Same thing about const and variable names :) trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10816> Extra empty line. trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10817> Extra empty lines. trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp <http://svn.reviewboard.kde.org/r/6339/#comment10818> Same thing about const and variable names. - Raphael On Jan. 16, 2011, 10:33 a.m., Cristi P wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/6339/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2011, 10:33 a.m.) > > > Review request for Kopete. > > > Summary > ------- > > Receive part: > a) protocol (as reading from various places) suggests that first a HEAD and > then GET should be done. Current code was asking them in parallel. > b) fixed the way sending the header was done. > > Sending: > a) changed to an async method of doing the send > b) send buffer size is dynamic to try to send as much as possible > c) ... various - made it work. > > Maybe fixes also #194833 although I'm not very sure what is that one about. > > > This addresses bugs and 242557. > https://bugs.kde.org/show_bug.cgi?id= > https://bugs.kde.org/show_bug.cgi?id=242557 > > > Diffs > ----- > > trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.cpp > 1214563 > trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/receivefiletask.h > 1214563 > trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.h > 1214563 > trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/sendfiletask.cpp > 1214563 > > Diff: http://svn.reviewboard.kde.org/r/6339/diff > > > Testing > ------- > > sending from kopete to pidgin and Yahoo messenger, and the other way around. > Sending was always failing for me. > (and posted a patch to pidgin/libpurple as well :-) ) > > > Thanks, > > Cristi > >
_______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel
