Re: New Framework Review: KDAV
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 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"
Re: New Framework Review: KDAV
On Friday, 19 June 2020 01:16:20 CEST Friedrich W. H. Kossebau wrote: > Am Samstag, 4. April 2020, 16:20:21 CEST schrieb Kevin Ottens: > > Overall apidox would likely need a big pass of cleanups as well. > > I locally prepared the addition of ECMAddQch usage for KDAV tonight, and > while testing the output already did some small bits of minor cleanup > (consistent casing of terms like URL, ETag, etc. in the texts), documenting > CamelCase includes of classes as well triggering the documentation of > namespace KDAV. Thanks! > One thing which should get fixed by someone with insights are all the > remaining references to some "ResourceBase::retrieveItems()" in the docs > (simply grep for the string to find those). That needs more context, or > better some generalization what is meant there (seems something-Akonadi > specific?). I've added the full namespace now, but you are right, this leaks details about the origin of all this and should probably be abstracted/generalized. Regards, Volker
Re: New Framework Review: KDAV
Am Freitag, 19. Juni 2020, 01:16:20 CEST schrieb Friedrich W. H. Kossebau: > Will commit the ECMAddQch stuff once https://invent.kde.org/pim/kdav/-/ > merge_requests/1 is in, to not block this more due to resulting new merge > conflicts. And while I was typing & sending, that got merged at the same time it seems. So landing now as well... Cheers Friedrich
Re: New Framework Review: KDAV
Am Samstag, 4. April 2020, 16:20:21 CEST schrieb Kevin Ottens: > Overall apidox would likely need a big pass of cleanups as well. I locally prepared the addition of ECMAddQch usage for KDAV tonight, and while testing the output already did some small bits of minor cleanup (consistent casing of terms like URL, ETag, etc. in the texts), documenting CamelCase includes of classes as well triggering the documentation of namespace KDAV. One thing which should get fixed by someone with insights are all the remaining references to some "ResourceBase::retrieveItems()" in the docs (simply grep for the string to find those). That needs more context, or better some generalization what is meant there (seems something-Akonadi specific?). Will commit the ECMAddQch stuff once https://invent.kde.org/pim/kdav/-/ merge_requests/1 is in, to not block this more due to resulting new merge conflicts. Cheers Friedrich
Re: New Framework Review: KDAV
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 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
Re: New Framework Review: KDAV
On Sun, Jun 14, 2020 at 8:03 PM Volker Krause 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. > > 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
Re: New Framework Review: KDAV
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? Thanks, Volker 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
Re: New Framework Review: KDAV
On Sun, May 24, 2020 at 8:52 AM 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?
Re: New Framework Review: KDAV
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/ >
Re: New Framework Review: KDAV
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 signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
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 signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
Hi, > In mostly all files it is not clear if the LGPL or the GPL is meant (stating > "GNU Library General Public License" and referring to the GPL text at the > end of the statement). This license statement is used for all files except > autotests/fakesever.cpp and autotests/fakeserver.h. > Luckily, from the copyright statements there are only 3 copyright holders: > Tobias, Sandro and Gregory. I put all 3 into CC. > > @Tobias, Sandro, Gregory: Can you clarify that the code you are holding > copyright about in this repository is licensed according to > LGPL-2.0-or-later? I'm fine with (re-)licensing this code to LGPL-2.0-or-later. Sandro signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
On Sonntag, 16. Februar 2020 10:29:42 CET Volker Krause wrote: > On Saturday, 15 February 2020 11:42:57 CET Andreas Cord-Landwehr wrote: > > Hi, sorry for this very late mail, missed the call for reviews... > > > > Would it be possible to do some license clarifications before moving kdav > > into the frameworks section? > > > > In mostly all files it is not clear if the LGPL or the GPL is meant > > (stating "GNU Library General Public License" and referring to the GPL > > text at the end of the statement). This license statement is used for all > > files except autotests/fakesever.cpp and autotests/fakeserver.h. > > Luckily, from the copyright statements there are only 3 copyright holders: > > Tobias, Sandro and Gregory. I put all 3 into CC. > > > > @Tobias, Sandro, Gregory: Can you clarify that the code you are holding > > copyright about in this repository is licensed according to > > LGPL-2.0-or-later? > > Thanks for checking! > > We have relicensing approval to LGPL for those three already in https:// > mail.kde.org/pipermail/kde-pim/2019-September/042912.html (thread continues > into the next month), so it's probably something I messed up while executing > the relicensing :) Great! Maybe the simplest way then is to directly replace all affected headers with the text "SPDX-License-Identifier: LGPL-2.0-or-later" and the ambiguity is gone :) Cheers, Andreas
Re: New Framework Review: KDAV
On Saturday, 15 February 2020 11:42:57 CET Andreas Cord-Landwehr wrote: > Hi, sorry for this very late mail, missed the call for reviews... > > Would it be possible to do some license clarifications before moving kdav > into the frameworks section? > > In mostly all files it is not clear if the LGPL or the GPL is meant (stating > "GNU Library General Public License" and referring to the GPL text at the > end of the statement). This license statement is used for all files except > autotests/fakesever.cpp and autotests/fakeserver.h. > Luckily, from the copyright statements there are only 3 copyright holders: > Tobias, Sandro and Gregory. I put all 3 into CC. > > @Tobias, Sandro, Gregory: Can you clarify that the code you are holding > copyright about in this repository is licensed according to > LGPL-2.0-or-later? Thanks for checking! We have relicensing approval to LGPL for those three already in https:// mail.kde.org/pipermail/kde-pim/2019-September/042912.html (thread continues into the next month), so it's probably something I messed up while executing the relicensing :) Regards, Volker signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
Hi, sorry for this very late mail, missed the call for reviews... Would it be possible to do some license clarifications before moving kdav into the frameworks section? In mostly all files it is not clear if the LGPL or the GPL is meant (stating "GNU Library General Public License" and referring to the GPL text at the end of the statement). This license statement is used for all files except autotests/fakesever.cpp and autotests/fakeserver.h. Luckily, from the copyright statements there are only 3 copyright holders: Tobias, Sandro and Gregory. I put all 3 into CC. @Tobias, Sandro, Gregory: Can you clarify that the code you are holding copyright about in this repository is licensed according to LGPL-2.0-or-later? Cheers, Andreas
Re: New Framework Review: KDAV
On Saturday, 9 November 2019 12:33:54 CET Volker Krause wrote: > Hi, > > 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). The rename to the KF5 namespace has been done, if more changes to the ABI are necessary now would be a good time to point those out, so we can integrate them before the 20.04 freeze :) Thanks, Volker > - release KDAV with 20.04 with the final API/ABI that the first KF5 release > will provide as well > - become part of the KF5 release in May or June 2020, release as a drop-in > replacement of the last application release > > In general this is following the same transition process that has been used > for Syndication, KHolidays, KContacts and KCalendarCore as that should cause > minimal disruptions for distributors, but if there's better ideas on how to > handle this, now is a good time to bring this up :) > > Feedback? > > Thanks, > Volker signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
вс, 10 нояб. 2019 г. в 15:44, David Faure : > > On samedi 9 novembre 2019 21:14:46 CET Alexander Potashev wrote: > > сб, 9 нояб. 2019 г. в 14:37, Volker Krause : > > > 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. > > > > Hi Volker, > > > > The name "KDAV" suggests that it might implement WebDAV. Do you think > > if it should be renamed to something like "DAVExtensions" or let's say > > "AnyDAV"? > > The name seems fine to me. The substring "DAV" appears just as much in WebDAV > as it does in CalDav/CardDav/GroupDav. So there's no reason to infer "WebDAV" > from "KDAV". > > "Extensions" sounds very optional, and "AnyDAV" looks like yet-another WebDAV- > based protocol in itself. > > The issue you see is that someone looking for WebDAV support might end up > thinking KDAV is the right thing to use? Well, maybe it should be, i.e. this > would probably be the right place for some proper future WebDAV API compared > to using kio_http with metadata directly OK. LGTM as well after your explanation. -- Alexander Potashev
Re: New Framework Review: KDAV
On samedi 9 novembre 2019 21:14:46 CET Alexander Potashev wrote: > сб, 9 нояб. 2019 г. в 14:37, Volker Krause : > > 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. > > Hi Volker, > > The name "KDAV" suggests that it might implement WebDAV. Do you think > if it should be renamed to something like "DAVExtensions" or let's say > "AnyDAV"? The name seems fine to me. The substring "DAV" appears just as much in WebDAV as it does in CalDav/CardDav/GroupDav. So there's no reason to infer "WebDAV" from "KDAV". "Extensions" sounds very optional, and "AnyDAV" looks like yet-another WebDAV- based protocol in itself. The issue you see is that someone looking for WebDAV support might end up thinking KDAV is the right thing to use? Well, maybe it should be, i.e. this would probably be the right place for some proper future WebDAV API compared to using kio_http with metadata directly -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: New Framework Review: KDAV
сб, 9 нояб. 2019 г. в 14:37, Volker Krause : > 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. Hi Volker, The name "KDAV" suggests that it might implement WebDAV. Do you think if it should be renamed to something like "DAVExtensions" or let's say "AnyDAV"? -- Alexander Potashev
Re: New Framework Review: KDAV
On Saturday, 9 November 2019 18:45:17 CET Christoph Feck wrote: > Hi Volker, > > On 11/09/19 12:33, 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. > > Could you clarify if the review is about > https://api.kde.org/kdepim/kdav/html/index.html or > https://api.kde.org/kdepim/kdav2/html/index.html ? good point, sorry about forgetting to add the corresponding links :) Code: https://phabricator.kde.org/source/kdav/ API docs: https://api.kde.org/kdepim/kdav/html/index.html Regards, Volker signature.asc Description: This is a digitally signed message part.
Re: New Framework Review: KDAV
Hi Volker, On 11/09/19 12:33, 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. Could you clarify if the review is about https://api.kde.org/kdepim/kdav/html/index.html or https://api.kde.org/kdepim/kdav2/html/index.html ? Christoph Feck
