Moving libkfacebook to extragear

[Pino Toscano]

Hi,

Alle sabato 27 ottobre 2012, Martin Klapetek ha scritto:
> I'd like to move libkfacebook, the foundation for akonadi-facebook
> resource, into extragear. It's been in use for a while, lots of
> distro ship it bundled with akonadi-facebook resource, which is now
> becaming part of kdepim-runtime for 4.10 with libkfacebook as
> optional compile-time dependency.
> 
> I'd like to ask for a review of this library, currently in kdereview
> - https://projects.kde.org/projects/kdereview/libkfacebook

Some notes:

- as others have pointed out, public classes should be better use 
d-pointers; for the jobs hierarchy, you could use a shared d-pointer,
see [1] for example

- GetCommentsJob::commentCount() could need the const qualifier

- LikeInfo::setCount() does not need const& for the int parameter

- ListJobBase::numEntries() (and in subclasses) could be better named
as entriesCount()

- ListJobBase, instead of a bool, could use an enum for readability

- in CommentInfo, NotificationInfo and PostInfo some methods return
*Info objects (or QList of them) while their setters take
QVariantMap/QVariantList; better just make them take the object too,
to not rely on the implementation (i.e. the parsing done with qjson)?
also some of those getters have a *Map() version too, maybe those
should be removed for the same reason above?

- in UserInfo there's birthdayAsString() while in other classes methods
like that are named fooString()

- UserInfo has no getters for city and country

- in UserInfo picture is a QUrl but website a QString?

- remember to remove the qjson includes from public headers

[1] http://techbase.kde.org/Policies/Library_Code_Policy/Shared_D-Pointer_Example

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121105/c741e75c/attachment.sig>