New Framework Review: KDAV
Volker Krause
vkrause at kde.org
Sat Jun 20 08:14:24 BST 2020
This has all been executed now, including the move on Gitlab and the necessary
changes to the dependency metadata. So unless I'm missing something this
should be all done now and we'll have KDAV in KF 5.72 as a drop-in replacement
for the one released with 20.04 :)
Thanks everyone for helping with this!
Volker
On Sunday, 14 June 2020 11:53:42 CEST Albert Astals Cid wrote:
> El diumenge, 14 de juny de 2020, a les 10:17:01 CEST, Ben Cooksley va
escriure:
> > On Sun, Jun 14, 2020 at 8:03 PM Volker Krause <vkrause at kde.org> wrote:
> > > With both 20.04.2 and 5.71.0 out I think it's now time to do this move.
> > >
> > > What extra steps do we need to take now that the framework/application
> > > distinction exists in Gitlab as well? I guess this is the first case of
> > > a
> > > post-migration move. Also, what is the impact on the 20.04.3 release
> > > when we move this in Gitlab?
> >
> > You'll need to file a Sysadmin ticket requesting we relocate the
> > repository from it's current home in pim/ to frameworks/
> > Gitlab will provide a redirect for this automatically, so existing
> > urls and clones won't be affected by this - although they will be
> > given a notice that it has moved.
> >
> > Systems such as scripty won't be impacted by this as they use the
> > stable repository identifier.
> >
> > In terms of the Release Service 20.04.3 release, Albert/Christoph will
> > need to comment on this.
>
> It shouldn't, the release service scripts don't care about the subdir repos
> are in, and given gitlab follows moves, we shouldn't not have a problem
> either with things like pushing tags, etc.
>
> Cheers,
> Albert
>
> > > Thanks,
> > > Volker
> >
> > Cheers,
> > Ben
> >
> > > On Sunday, 24 May 2020 08:52:17 CEST Volker Krause wrote:
> > > > The remaining issues that didn't change ABI anymore (movable value
> > > > types,
> > > > hide private methods/slots inside the private classes, etc) have long
> > > > since
> > > > been addressed.
> > > >
> > > > I think there's two possible time slots to actually execute the move
> > > > to
> > > > frameworks now:
> > > > * ASAP, for the June release.
> > > > * For the July release, just in time for the 20.08 dependency freeze.
> > > >
> > > > Opinions?
> > > >
> > > > Thanks,
> > > > Volker
> > > >
> > > > On Saturday, 4 April 2020 17:32:19 CEST Volker Krause wrote:
> > > > > Thanks for the review! We are cutting it close again with the 20.04
> > > > > deadline, but fortunately most of these findings aren't ABI-breaking
> > > > > :)
> > > > >
> > > > > The result was discussed in more detail at the (virtual) PIM sprint,
> > > > > summary below for the record.
> > > > >
> > > > > On Saturday, 4 April 2020 16:20:21 CEST Kevin Ottens wrote:
> > > > > > 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;
> > > > >
> > > > > Cosmetic with no ABI impact, we can do that post 20.04 still.
> > > > >
> > > > > > * Is it worth making DavCollection moveable? It's only copyable
> > > > > > right
> > > > > > now;
> > > > >
> > > > > Probably yes, that's new API with no ABI break, so we can do that
> > > > > post
> > > > > 20.04 as well.
> > > > >
> > > > > > * 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);
> > > > >
> > > > > I have no idea what ctag is (I am only doing the technical work
> > > > > needed to
> > > > > turn this into a framework, I didn't write this library).
> > > > >
> > > > > > * 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);
> > > > >
> > > > > I think this is to be able to control which properties get changed,
> > > > > rather
> > > > > than sending the full set of them.
> > > > >
> > > > > > * 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?
> > > > >
> > > > > Fixed in https://phabricator.kde.org/D28564 and
> > > > > https://phabricator.kde.org/ D28566
> > > > >
> > > > > > * DavCollectionsMultiFetchJob is inconsistent as it's not using
> > > > > > Q_DECLARE_PRIVATE;
> > > > >
> > > > > That's due to using KJob as a base directly.
> > > > >
> > > > > Subsequent discussion suggested this should be a KCompositeJob,
> > > > > David is
> > > > > taking care of this.
> > > > >
> > > > > > * KDAV::Error would benefit from more apidox;
> > > > >
> > > > > Yes, not blocked by the 20.04 freeze though.
> > > > >
> > > > > > * Is it worth making DavItem moveable? It's only copyable right
> > > > > > now;
> > > > >
> > > > > See above, same as DavCollection.
> > > > >
> > > > > > * Same comment about etag for DavItem than the ctag one for
> > > > > > DavCollection
> > > > >
> > > > > See above, same as ctag.
> > > > >
> > > > > > * 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);
> > > > >
> > > > > ABI impact mitigated by https://phabricator.kde.org/D28562 so we can
> > > > > clean
> > > > > this up after 20.04.
> > > > >
> > > > > > * 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);
> > > > >
> > > > > Also fixed by https://phabricator.kde.org/D28562.
> > > > >
> > > > > > * Make DavUrl moveable?
> > > > >
> > > > > See above, same as DavCollection and DavItem.
> > > > >
> > > > > > * EtagCache probably shouldn't have anything protected, also, why
> > > > > > is it
> > > > > > a
> > > > > >
> > > > > > QObject at all?
> > > > >
> > > > > This is why:
> > > > > https://lxr.kde.org/source/kde/pim/kdepim-runtime/resources/dav/
> > > > > resource/akonadietagcache.cpp
> > > > >
> > > > > > * Are we sure we want to return a QLatin1String in ProtocolInfo?
> > > > > > this
> > > > > >
> > > > > > strike me as an odd choice.
> > > > >
> > > > > Fixed in https://phabricator.kde.org/D28563.
> > > > >
> > > > > > Overall apidox would likely need a big pass of cleanups as well.
> > > > > >
> > > > > > I think that's it from me.
> > > > >
> > > > > I hope we managed to address everything on short notice that would
> > > > > require
> > > > > ABI breaks after the 20.04 release (and thus cause a delay of the
> > > > > frameworks move Volker
More information about the release-team
mailing list