----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/6474/#review10000 -----------------------------------------------------------
Great work, this is something that has been quite a while on my TODO list, but never managed to start working on it. Passwords are not being saved to the remote storage at all, though is it really the place to store them instead of using kwallet? Anyways, the problem with password not getting ever passed to bookmarks I think is located in jabberclient.cpp:852 which gets called from jabberaccount.cpp:1567. No idea how to handle this though, the whole system (inserting bookmarks automatically on join) doesn't feel quite right for me. Otherwise this is good, weirdly I didn't find any bugs related to the bookmark manager from bko.. Anyways, this is an awesome patch, thank you a lot :-) I'd say ship it, but perhaps someone else could also check this. branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.h <http://svn.reviewboard.kde.org/r/6474/#comment11202> HACKING states the tabs are used for indentation, but you're using two spaces here (and elsewhere too). branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.h <http://svn.reviewboard.kde.org/r/6474/#comment11201> Indent issue? branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.cpp <http://svn.reviewboard.kde.org/r/6474/#comment11203> Whitespace not needed? branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.cpp <http://svn.reviewboard.kde.org/r/6474/#comment11204> Whitespace not needed for () I think. branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.cpp <http://svn.reviewboard.kde.org/r/6474/#comment11205> If 'if ( something() )' is the correct style for Kopete srcs, append the space here too. branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.cpp <http://svn.reviewboard.kde.org/r/6474/#comment11208> Using SelectAction for Edit bookmarks isn't very nice here, but I think the feature is so important that this can be fixed later... branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/ui/dlgjabberbookmarkeditor.cpp <http://svn.reviewboard.kde.org/r/6474/#comment11207> whitespace - Teemu On Feb. 11, 2011, 2:16 p.m., Tobias Koenig wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/6474/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2011, 2:16 p.m.) > > > Review request for Kopete. > > > Summary > ------- > > This patch provides an editor for jabber group chat bookmarks. You can easily > remove or rename the bookmarks and (the most important thing IMHO ;)) toggle > the auto-join feature of a group chat. > > > Diffs > ----- > > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/CMakeLists.txt 1219908 > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.h > 1219908 > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/jabberbookmarks.cpp > 1219908 > > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/ui/dlgjabberbookmarkeditor.h > PRE-CREATION > > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/ui/dlgjabberbookmarkeditor.cpp > PRE-CREATION > > branches/KDE/4.6/kdenetwork/kopete/protocols/jabber/ui/dlgjabberbookmarkeditor.ui > PRE-CREATION > > Diff: http://svn.reviewboard.kde.org/r/6474/diff > > > Testing > ------- > > > Thanks, > > Tobias > >
_______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel
