[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] Review Request 117812: Port from QJSON to QtCore JSON classes
From:       "Kevin Krammer" <krammer () kde ! org>
Date:       2014-09-16 18:27:05
Message-ID: 20140916182705.22584.88071 () probe ! kde ! org
[Download RAW message or body]



> On Sept. 15, 2014, 4:43 nachm., Kevin Krammer wrote:
> > akonadi-socialutils/src/serializer/akonadi_serializer_socialfeeditem.cpp, line 68
> > <https://git.reviewboard.kde.org/r/117812/diff/1/?file=268828#file268828line68>
> > 
> > @Milian: changing the API of SocialFeedItem would "encode" a specific \
> > serialization in the data types API, no? 
> > @Martin: it is still a JSON document, right with the same structure, no?
> 
> Milian Wolff wrote:
> If this is public API, then don't change it. If it's internal API then you could \
> change it as long as you ensure the serialization stays the same between all \
> involved apps. I don't know anything about this codebase, just wanted to mention \
> that the QJson <-> QVariant conversion is relatively costly. But maybe/probably you \
> can ignore this here?! 
> Kevin Krammer wrote:
> It is API in the feed item class, but I have no idea if or how it is used. Martin?
> 
> Martin Klapetek wrote:
> It's a data container used to store posts from social networks, so it's used by the \
> facebook resource (plus there's also a resource for twitter and identica, somewhere \
> in scratch repos). Then it's also used for retrieving and manipulating social posts \
> in the clients (sadly currently there's none though). 
> I don't mind that change at all; the itemSourceMap is basically the raw JSON data \
> retrieved from the server, imo maybe actually it would be better to keep it a \
> QJsonObject. 
> Kevin Krammer wrote:
> So the remote format is always JSON?
> sounds a bit like an assumption to me, but maybe there is a standard for these \
> things :) 
> Martin Klapetek wrote:
> If I'm not mistaken, JSON is mandatory for OAuth. And most of the services use \
> OAuth. So I guess it makes sense to stick to only one data format and not force \
> developers to work with multiple... *shrug* 
> However, we can simply force the JSON format on anything that does not use it \
> (Facebook, Twitter and Pump.io all do), it should be like 1 in 100 anyway. 
> Martin Klapetek wrote:
> Actually, this field can also stay empty. So if something does not send out JSON, \
> we can just leave it. It's just a copy of the raw data for "in case it might be \
> handy" (I was using it in the SocialFeed for Twitter for getting some additional \
> metadata).

If it is a copy of the raw data and only few clients every need it, would this be a \
candidate for a different payload part?


- Kevin


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


On Sept. 13, 2014, 12:38 nachm., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117812/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2014, 12:38 nachm.)
> 
> 
> Review request for KDEPIM-Libraries and Martin Klapetek.
> 
> 
> Repository: kdepimlibs
> 
> 
> Description
> -------
> 
> Port akonadi-socialutils from QJSON to QtCore JSON classes
> 
> 
> port away from Q_EXPORT_PLUGIN2
> 
> 
> Diffs
> -----
> 
> CMakeLists.txt 7a775f8de4c078993809f42777b7eb8e324daca0 
> akonadi-socialutils/CMakeLists.txt af0d37beaf649c26eba02117f2b13ddfa74dc81d 
> akonadi-socialutils/cmake/FindQJSON.cmake 9701c81131895bfcae0937cef1dc440578729117 
> akonadi-socialutils/src/CMakeLists.txt db34d23796418949f19a87cd3fe0cfe2d0e9d6e2 
> akonadi-socialutils/src/serializer/akonadi_serializer_socialfeeditem.h \
> ac292d39472be237a635355eaaaa1a663a4b8b17  \
> akonadi-socialutils/src/serializer/akonadi_serializer_socialfeeditem.cpp \
> 4816afafbcb68b95023f061b265015b60b8073e2  akonadi/src/core/searchquery.cpp \
> 16c67687ebb682d144294f0c1f2a8dac6263e6fc  akonadi/tests/CMakeLists.txt \
> e2a4e2423c994b1e65fe89fc0a7f93c690688dee  cmake/modules/CMakeLists.txt \
> 2f5b8844c7fb1bb66ab5420b9fb3080f6c96c7a3  cmake/modules/FindQJSON.cmake \
> 9701c81131895bfcae0937cef1dc440578729117  
> Diff: https://git.reviewboard.kde.org/r/117812/diff/
> 
> 
> Testing
> -------
> 
> compiles
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
> 

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


[prev in list] [next in list] [prev in thread] [next in thread]