----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106575/#review19420 -----------------------------------------------------------
Not bad. Some comments below. ../../defaults/chat-widget.h <http://git.reviewboard.kde.org/r/106575/#comment15349> Both these should be private ../../defaults/chat-widget.cpp <http://git.reviewboard.kde.org/r/106575/#comment15350> You've got a bug here. I talk to Martin on English (my default), I change to German. now I change back to English. This will not save anything as English is my default language, and I'll be still on German next time I load. Doing an "if dict != locale()->language()" is a very good idea, but it needs an " else { removeEntry...() " Also why bother checking the config group exists? That sounds a wrong thing to do. ../../defaults/chat-widget.cpp <http://git.reviewboard.kde.org/r/106575/#comment15351> You already have the configGroup from two lines earlier (also memory leak) - David Edmundson On Sept. 25, 2012, 7:52 p.m., Nick Lou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106575/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2012, 7:52 p.m.) > > > Review request for Telepathy. > > > Description > ------- > > Fix for the reported Bug 291764 (Set spellchecking language per contact > instead of per tab). > If a contact changes the default spellchecking option to one of his/her > choice, > the new option is saved in a file as a pair, target Id and spell checking > option, > where the target Id is a group and option value has a key named "language". > > > This addresses bug 291764. > http://bugs.kde.org/show_bug.cgi?id=291764 > > > Diffs > ----- > > ../../defaults/chat-widget.h 7d2dcf2 > ../../defaults/chat-widget.cpp 6c9c7ed > ../../defaults/chat-window.cpp eb1b7a0 > > Diff: http://git.reviewboard.kde.org/r/106575/diff/ > > > Testing > ------- > > Tested it by changing the spellchecking option of various pairs, > and then by closing and reopening their chat tabs and checking the values. > I also checked the values stored in the config file. > > > Thanks, > > Nick Lou > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
