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

[Martin Klapetek]

> On Feb. 17, 2014, 3: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.
> 
> Kevin Krammer wrote:
>     @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

Well they do in terms of user input, painting events are still processed, yes.


- Martin


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


On Feb. 17, 2014, 4: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, 4: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/