----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115425/#review49227 -----------------------------------------------------------
I think this patch is mostly sensible. (except for my code comment) In Xeng's change [2] the config option stores if now playing is currently enabled. "Enable option now behaves as "enable on login", and changes at contact list will be temporary for this login. And use requested presence to evaluate the new presence by this plugin." Having a code path that sets it to true but not one that sets it to false is wrong, which is what this patch fixes. Martin, I think there's some confusion because this differs from what your original idea of what that setting does, which is what I thing you are talking about in your review comment. I still not sure as to how you think it should behave despite [1] having been open for over a year. [1] https://bugs.kde.org/show_bug.cgi?id=307582 [2] https://git.reviewboard.kde.org/r/113066/. global-presence-chooser.cpp <https://git.reviewboard.kde.org/r/115425/#comment34762> Then we don't need lines 337/338 as we end up with a code path where we set it twice. - David Edmundson On Feb. 3, 2014, 3 p.m., James Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115425/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2014, 3 p.m.) > > > Review request for Telepathy and Martin Klapetek. > > > Repository: ktp-contact-list > > > Description > ------- > > Enables / disables Now Playing in systemsettings every time it is enabled / > disabled in the contact list. > > Fixes systemsettings kcm showing nowplaying enabled while the contact list > has disabled its functionality. > > > Diffs > ----- > > global-presence-chooser.cpp 2047473 > > Diff: https://git.reviewboard.kde.org/r/115425/diff/ > > > Testing > ------- > > Compile, run. > > > Thanks, > > James Smith > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
