'Re: New Framework Review: KDAV'

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

List:       kde-frameworks-devel
Subject:    Re: New Framework Review: KDAV
From:       Albert Astals Cid <aacid () kde ! org>
Date:       2020-06-14 9:53:42
Message-ID: 8250265.NQ4CqxE24q () xps
[Download RAW message or body]

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@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
> > 
> > 
> > 
> > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic