[Kde-pim] Review Request 115769: Add KAccounts supoprt to the facebook resource

[Kevin Krammer]

> On Feb. 17, 2014, 2:57 p.m., Martin Klapetek wrote:
> > resources/facebook/facebookresource.cpp, line 99
> > <https://git.reviewboard.kde.org/r/115769/diff/4/?file=244520#file244520line99>
> >
> >     I wonder if this will block all of Akonadi...(it communicates over DBus internally, right?)
> 
> Kevin Krammer wrote:
>     This is very likey a KJob::exec(), A.K.A. a nested event loop. Thus it would not block anything.
>     Still something that might be worth trying to avoid, nested event loops are evil (tm)!
> 
> Martin Klapetek wrote:
>     Actually given the resources run in their own process, this would block only the UI spawned by this resource, which is ok(~ish).
> 
> Àlex Fiestas wrote:
>     As far as I know this is ok, I can change it nevertheless, will be ok if I justm ove that code into a slot? or I need to do something else akonadi-wise?
>     
>     Keep in mind that I'm just rebasing this, I'm not the author of the patch so I know little about how a resource works.
> 
> Àlex Fiestas wrote:
>     The configuration ui won't be shown if configured by accounts-sso.

@Martin: nested event loops do not block the UI
@Alex: configureByAccount doesn't sound like an Akonadi specific function, so I doubt that anything in the resource expects all work to be done when it returns. Guess from its name it is actually something related to the accounts system


- Kevin


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


On Feb. 17, 2014, 3:22 p.m., Àlex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115769/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2014, 3:22 p.m.)
> 
> 
> Review request for KDEPIM and Martin Klapetek.
> 
> 
> Repository: kdepim-runtime
> 
> 
> Description
> -------
> 
> Patch mostly made by mklapetek that makes use of webaccounts if present.
> 
> 
> Diffs
> -----
> 
>   resources/facebook/facebookresource.cpp 5f59018 
>   resources/facebook/facebookresource.desktop 80cec32 
>   resources/facebook/facebookresource.h 1b53f9a 
>   resources/facebook/settingsbase.kcfg 570a1fc 
>   resources/facebook/settingsdialog.cpp fc97f6d 
>   resources/shared/getcredentialsjob.h PRE-CREATION 
>   resources/shared/getcredentialsjob.cpp PRE-CREATION 
>   CMakeLists.txt 5f2ba59 
>   resources/facebook/CMakeLists.txt f24de9a 
> 
> Diff: https://git.reviewboard.kde.org/r/115769/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Àlex Fiestas
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/