> On Jan. 21, 2014, 11:13 p.m., Martin Klapetek wrote:
> > contact-cache.cpp, line 52
> > <https://git.reviewboard.kde.org/r/115060/diff/1/?file=234176#file234176line52>
> >
> >     If you have your facebook enabled and one or more accounts, this might 
> > get really slow for just one contact as it will re-insert every contact 
> > because of just one. Would it be worth actually caching the ID and checking 
> > before inserting? ID should be enough because that would give you just a 
> > couple of re-inserts, compared to hundreds.

We need to update all data (alias and avatarToken) for each "new" contact.
Will exists code add new record for same contactId if it have change avatar?
What is faster — to do manual check all data before insertion, or to add some 
key column and let DB check it for us?


> On Jan. 21, 2014, 11:13 p.m., Martin Klapetek wrote:
> > contact-cache.cpp, line 48
> > <https://git.reviewboard.kde.org/r/115060/diff/1/?file=234176#file234176line48>
> >
> >     ...and the coding style! ;)
> >     
> >     In constructor too.

I'm working on it. Fixed.


> On Jan. 21, 2014, 11:13 p.m., Martin Klapetek wrote:
> > contact-cache.h, line 2
> > <https://git.reviewboard.kde.org/r/115060/diff/1/?file=234175#file234175line2>
> >
> >     Remove or fill in

I'm currently working on second version of this patch. Fixed.


> On Jan. 21, 2014, 11:13 p.m., Martin Klapetek wrote:
> > contact-cache.h, line 6
> > <https://git.reviewboard.kde.org/r/115060/diff/1/?file=234175#file234175line6>
> >
> >     As all of ktp is LGPL, change to LGPL too..?

Fixed here, but there is a lot of license mixing in ktp code. E.g. libkpeople 
have 16 files under GNU GPL and 51 file under GNU LGPL.


> On Jan. 21, 2014, 11:13 p.m., Martin Klapetek wrote:
> > contact-cache.cpp, line 45
> > <https://git.reviewboard.kde.org/r/115060/diff/1/?file=234176#file234176line45>
> >
> >     either put space between arguments in SIGNAL and SLOT or put no space 
> > in both places ;) Actually I think there should be no space for optimization

Fixed.


- Alexandr


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115060/#review47901
-----------------------------------------------------------


On Jan. 17, 2014, 2:48 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115060/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 2:48 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> -------
> 
> Save all persistent information on a contact to a database
> 
> In KPeople we need the name + avatar of a person even when the
> user is offline. The only approach is to have a small cache.
> 
> This is apparently being merged into Telepathy at some point,
> so we maybe remove this in future
> 
> ----
> 
> Won't merge till we make the relevant change in the kpeople plugin too.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt a31245b 
>   contact-cache.h PRE-CREATION 
>   contact-cache.cpp PRE-CREATION 
>   contactnotify.cpp 314d48a 
>   telepathy-module.cpp a754283 
> 
> Diff: https://git.reviewboard.kde.org/r/115060/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to