New Framework Review: KDAV

[Kevin Ottens]

Hello,

On Saturday, 9 November 2019 12:33:54 CEST Volker Krause wrote:
> during Akademy there was a request to promote KDAV from KDE PIM to
> Frameworks for use by Plasma Mobile. KDAV is a framework that implements
> the CalDav/ CardDav/GroupDav protocol on top of KIO's WebDav support. It
> would be classified as a functional tier 3 framework.
> 
> So far we have fixed a number of obvious ABI-compatibility issues, removed
> QtXml[Patterns] usage from the public interface and relicensed GPL parts
> (apart from a bit of test code) to LGPL. The next step would be a more
> thorough review to identify changes necessary before becoming a Framework.
> 
> To avoid the last minute invasive changes we ended up doing for
> KCalendarCore, I'd propose the following timeline:
> 
> - identify and implement all necessary changes to the API and ABI until the
> 20.04 Application release (that includes the still necessary move to the KF5
> library namespace).

I'm likely late to the party, but here is what I found by looking at KDAV 
master today (first day of the KDE PIM sprint):
 * There's a few private methods or Q_SLOTS that I'd hide completely by moving 
them to the d-pointer, for the slots we're using type safe connects so they 
don't even need to be marked as slots at all;
 * Is it worth making DavCollection moveable? It's only copyable right now;
 * We might want to do something about "ctag" in DavCollection it's a bit 
obscure as a name (and the API doc doesn't help), also it seems to not be an 
official standard (while being widely supported) and there's the sync-token 
mechanism which has a RFC (RFC6578);
 * Why isn't DavCollectionModifyJob using DavCollection somehow? (might just 
be my ignorance but I find it surprising that it is solely based on a property 
mechanism);
 * DavCollections(Multi)FetchJob has a mysterious "protocol" parameter on its 
collectionDiscovered signal, is it really necessary? if it has to stay, 
shouldn't be at least documented? or at least a safer type than int?
 * DavCollectionsMultiFetchJob is inconsistent as it's not using 
Q_DECLARE_PRIVATE;
 * KDAV::Error would benefit from more apidox;
 * Is it worth making DavItem moveable? It's only copyable right now;
 * Same comment about etag for DavItem than the ctag one for DavCollection
 * I'd be tempted to move all the protected methods of DavJobBase on its d-
pointer, the job subclasses would have access to them anyway, it'd make sense 
to put them protected in the header only if we expect subclasses outside of 
the lib (and I doubt this is actually supported);
 * It needs to decide between Qt smart pointers or STL ones I think, found a 
bit of both so far (I'd lean toward STL ones but maybe that's just me);
 * Make DavUrl moveable?
 * EtagCache probably shouldn't have anything protected, also, why is it a 
QObject at all?
 * Are we sure we want to return a QLatin1String in ProtocolInfo? this strike 
me as an odd choice.

Overall apidox would likely need a big pass of cleanups as well.

I think that's it from me.

Regards.
-- 
Kevin Ottens, http://ervin.ipsquad.net

enioka Haute Couture - proud patron of KDE, https://hc.enioka.com/en
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/release-team/attachments/20200404/4df45835/attachment.sig>