> On June 25, 2013, 6:17 a.m., Pali Rohár wrote: > > Hello, nice work :-) > > > > Some my comments: > > * please make all coding style changes in separate git commit (it will be > > easier to review and read your patch) > > * also if you can move crash during key generation to separate git commit > > * and move also patch for unencrypted messages to separate git commit > > * this patch dropping libotr 3.x support, but lot of linux distributions > > does not have libotr 4.x - can you update patch to support both versions? I > > do not want to see that linux distributions drop otr support in kopete > > because they do not have needed libraries... > > > > For adding new strings, you need to contact kde-i18n-doc mailinglist and > > ask for it.
Now read the description again. > * please make all coding style changes in separate git commit (it will be > easier to review and read your patch) As the code was really badly formatted (no - we didn't have reviewboard the time I wrote the plugin initially) and QtCreator automatically trims spaces at the end of lines, I'm not going to manually add whitespaces at the end of the lines to break that again. > * also if you can move crash during key generation to separate git commit I don't think it makes sense to split the fix of the crash into a separate commit. Especially that code changes during the plugin porting anyways. > * and move also patch for unencrypted messages to separate git commit There is no patch for unecrypted messages. That was a change from libotr 3.2 to libotr 4.0. > * this patch dropping libotr 3.x support, No it is not. If you would have read at least the description you would see that it is tested and works with Adium, which does only support OTR protocol version v2. Heck, it even supports OTR protocol v1. > For adding new strings, you need to contact kde-i18n-doc mailinglist and ask > for it. You mean to ask for an exception for getting it inot 4.11? - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111227/#review35015 ----------------------------------------------------------- On June 24, 2013, 10:49 p.m., Michael Zanetti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111227/ > ----------------------------------------------------------- > > (Updated June 24, 2013, 10:49 p.m.) > > > Review request for Kopete. > > > Description > ------- > > This patch ports kopete-otr to libotr 4.0.0 > > Changes: > -> support for translating libotr-generated messagess. > -> incoming unencrypted messages during an ecrypted session can now be > threated as normal messages producing a notification and saved to history > (Bug 204502) > -> Along with the port I worked around the crash with incoming messages > during key generation (Bugs 195328, 218570, 298681, 304105, 306276, 309987, > 318255) > -> Multiple chat session to the same user with different resources can now be > distinguished > > Because the original code was rather badly formatted the change also contains > some formatting changes which I'm not sure what exactly is the difference. I > tried to make as few coding style changes as possible. > > > Oh, and it does add some new strings, but shouldn't change any existing ones. > The ones newly added are the ones that have been inside libotr before - > hardcoded to english. So even if we would ship this patch without updating > translations, translation wise it would look the same as before. So, do you > guys think we could squeeze it into 4.11 still? > > > This addresses bugs 195328, 204502, 218570, 298681, 304105, 306276, 309987, > and 318255. > http://bugs.kde.org/show_bug.cgi?id=195328 > http://bugs.kde.org/show_bug.cgi?id=204502 > http://bugs.kde.org/show_bug.cgi?id=218570 > http://bugs.kde.org/show_bug.cgi?id=298681 > http://bugs.kde.org/show_bug.cgi?id=304105 > http://bugs.kde.org/show_bug.cgi?id=306276 > http://bugs.kde.org/show_bug.cgi?id=309987 > http://bugs.kde.org/show_bug.cgi?id=318255 > > > Diffs > ----- > > CMakeLists.txt 18896be > cmake/modules/FindLibOTR.cmake e9dbf31 > plugins/otr/authenticationwizard.h a4dea40 > plugins/otr/authenticationwizard.cpp e4d5d13 > plugins/otr/kopete_otr.desktop 2429868 > plugins/otr/otrguiclient.h 0347075 > plugins/otr/otrguiclient.cpp 11329db > plugins/otr/otrlchatinterface.h 5bb6e1d > plugins/otr/otrlchatinterface.cpp 56ffd0c > plugins/otr/otrlconfinterface.h 0cfdf29 > plugins/otr/otrlconfinterface.cpp 7d58462 > plugins/otr/otrplugin.h 35ecb7b > plugins/otr/otrplugin.cpp ef973ed > plugins/otr/otrpreferences.h 7aa742c > plugins/otr/otrpreferences.cpp ece403a > plugins/otr/privkeypopup.h 4b271e9 > plugins/otr/privkeypopup.cpp 3fea9e2 > > Diff: http://git.reviewboard.kde.org/r/111227/diff/ > > > Testing > ------- > > Tested with XMPP against Pidgin (OTRv3) and Adium (OTRv2). The overall > handling of messages (escaping, parsing etc) did not change with this patch, > so it shouldn't break any other protocol. Still, some more testing wouldn't > hurt if you have some other protocols in use you can test with. > > > Thanks, > > Michael Zanetti > >
_______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel
