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

Martin Klapetek martin.klapetek at gmail.com
Mon Feb 17 10:57:40 GMT 2014


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



CMakeLists.txt
<https://git.reviewboard.kde.org/r/115769/#comment35135>

    We actually don't check anywhere if we've found this or not, so if you don't have this installed, the build will fail (as it will fail to build getcredentialsjob.cpp). So we either need to make it required (probably not) or have some checks around (I think the facebook/CMakeLists.txt should be enough)



resources/shared/getcredentialsjob.h
<https://git.reviewboard.kde.org/r/115769/#comment35134>

    The rest of kdepim has "at KDE e.V's discretion" here instead...not sure if a problem or not though.



resources/shared/getcredentialsjob.cpp
<https://git.reviewboard.kde.org/r/115769/#comment35130>

    The pointer signs should be next to the variable (it's correct in the .h)



resources/shared/getcredentialsjob.cpp
<https://git.reviewboard.kde.org/r/115769/#comment35132>

    SignOn::Identity* identity -> SignOn::Identity *identity



resources/shared/getcredentialsjob.cpp
<https://git.reviewboard.kde.org/r/115769/#comment35133>

    Do we need this printed if we're doing setErrorText (which I assume will get displayed somewhere)? The less debug output the better



resources/shared/getcredentialsjob.cpp
<https://git.reviewboard.kde.org/r/115769/#comment35131>

    Files should have an empty line at the end (.h file too)


- Martin Klapetek


On Feb. 17, 2014, 11:48 a.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, 11:48 a.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
> -----
> 
>   CMakeLists.txt 5f2ba59 
>   resources/facebook/CMakeLists.txt f24de9a 
>   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 
> 
> 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/


More information about the kde-pim mailing list