Created attachment 66088 [details] [review] Proposed patch Hi, I've got an upstream request for being able to disable logging for specific contact. I've noticed that there is already some disabled API for this in logger, so I tried to provid a basic implementation. Maybe (probably) this is not exactly what you had in mind, but if you tell me what to do better or point me the right direction, I'll work on it :)
Created attachment 66089 [details] [review] Proposed patch Slightly better patch without wrong files and leftover changes...sorry, it's quite late here :)
I'll do proper review next week, meanwhile take note that you'll have to provide unit test for your new feature. About that feature, is there any use of it that you would like ? Is there any proposal for integration into Empathy ?
Comment on attachment 66089 [details] [review] Proposed patch Review of attachment 66089 [details] [review]: ----------------------------------------------------------------- Why do you convert the string arrays into GSList ? Also, you could enable regex, this would make your filter more flexible. ::: telepathy-logger/conf.c @@ +261,5 @@ > +_tpl_conf_set_accounts_ignorelist (TplConf *self, > + GSList *newlist) > +{ > + GSList *iter; > + gint ii, len; Any reason for ii, why not just i ?
(In reply to comment #2) > I'll do proper review next week, meanwhile take note that you'll have to > provide unit test for your new feature. I'll provide the unit tests during the week (must learn how to do it first :) > About that feature, is there any use of > it that you would like ? Is there any proposal for integration into Empathy ? We would like to have it in KDE Telepathy (In reply to comment #3) > Why do you convert the string arrays into GSList ? No particular reason, I just think it's more programmer-friendly. But now I realize the TplConf API is not public so it makes no sense really....Which rises a question in my mind - how will users change the values - is direct changing of the GSettings property OK? > Also, you could enable regex, this would make your filter more flexible. OK. > > ::: telepathy-logger/conf.c > @@ +261,5 @@ > > +_tpl_conf_set_accounts_ignorelist (TplConf *self, > > + GSList *newlist) > > +{ > > + GSList *iter; > > + gint ii, len; > > Any reason for ii, why not just i ? No, just a habit :)
(In reply to comment #4) > No particular reason, I just think it's more programmer-friendly. But now I > realize the TplConf API is not public so it makes no sense really....Which > rises a question in my mind - how will users change the values - is direct > changing of the GSettings property OK? I think so, the enable/disable feature have been there for quite a while and it seems it always been accessed through the (originally GConf) GSettings API. My only concern in this case is that one can slighly corrupt the list. I'd say it's your call, if it's direct access, then we should find a way to document that very well.
Created attachment 67260 [details] [review] Updated patch Sorry for the delay, too much work :( Updated patch against latest master - implements _tpl_conf_is_account_ignored() - _tpl_conf_{get,set}_accounts_ignorelist() don't use GSLists anymore - added tests (a bit clumsy I'd say, but work :-) - coding style fixes
Uhm, ping? :-)
Created attachment 76995 [details] [review] Updated patch Hi, I'm back again :-) I've added a high-level API to TplLogManager to manage the ignorelist. I believe this way is better than exposing the gconf values to users. Tests have been updated accordingly.
Will be in next development release, 0.9.0. commit c0a2104a590e9e68226f836d22ff6e645f3e33d3 Author: Dan Vrátil <dvratil@redhat.com> Date: Thu Apr 4 11:24:21 2013 -0400 Add ignore list capability This allow disabling logging for a specific contact. Fix bug https://bugs.freedesktop.org/show_bug.cgi?id=54033
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.