[Kde-pim] Request for merging akonadi-social-utils into kdepimlibs

[Allen Winter]

On Thursday 20 September 2012 07:57:14 PM Martin Klapetek wrote:
> Hi,
> 
> during this year's GSoC I've been working on Social Feed[1], during which I
> wrote several libraries. One of them is akonadi-social-utils [2], which
> contains shared stuff for the 'social' resources, currently it contains:
>  * Image provider - fetches images from web and stores them in local cache
> (avatars and social net post's images)
>  * SocialFeedItem - unified data structure for use in the Social Feed
>  * SocialFeedItem serializer - de-/serializes the SocialFeedItem
>  * SocialNetworkAttributes - these are special attributes for social
> collections in Akonadi, currently it sets which signed-in username that
> collection belongs to, the social network name, if it can be posted to this
> network(/collection) and max post length (140 for twitter etc).
> 
> This library is currently at kde:scratch/mklapetek/akonadi-social-utils (or
> use [2]).
> 
> It also contains tests, currently only for Image Provider.
> 
> I'd like to request a review of this library and advice on how to proceed
> with the merge. Please ask if you have any questions.
> 

builds and installs ok <check>
no compile warnings <check>
test passes <check>

missing Messages.sh (but ok since there are no i18n strings)
missing Mainpage.dox

licensing is mostly fine.  please add a copyright and/or license to
 libakonadisocialutils_export.h
 cmake/FindQJSON.cmake
(wait! cmake has a FindQJSON, can't you use that one?  There's a FindQJSON.cmake in
 kdeplasma-addons, kde-workspace, kdevplatform, amarok... I wonder why?)

there are a couple of very minor krazy issues (spelling, file doesn't end with newline) but no big deal

the style on a lot of the code is not compliant with our standard, but again no big deal.
I'll want to fix that at some point though.
See http://community.kde.org/KDE_PIM/Development/CodingStyle

Please remove the REQUIRED argument for QJSON in the top-level CMakeLists.txt
as the following line with macro_log_feature() will handle bailing out for you as needed.

You don't need the find_package(QJSON REQUIRED) in the serializer/CMakeLists.txt do you?
Since already checked at the top-level

In general: this library looks fine and I have no objections to adding it to kdepimlibs.
Would be a very nice addition.

-Allen


> [1] -
> http://martys.typepad.com/blog/2012/09/late-gsoc-wrap-up-social-feed.html
> [2] -
> http://quickgit.kde.org/index.php?p=scratch%2Fmklapetek%2Fakonadi-social-utils.git&a=summary
> 
> PS: I just noticed that SocialNetworkAttributes misses comments, so I will
> add them later tonight.
> 
> Cheers
> 
_______________________________________________
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/