mpyne requested changes to this revision.
mpyne added a comment.
This revision now requires changes to proceed.
Still a couple of things:
1. The included new autotest fails for me, because of using `OrLaterVersion`
as a default version restriction. `OnlyThisVersion` is used elsewhere as the
default, so I recommend fixing the KAboutLicense::Private to match the
autotest. I have verified that all KAboutData autotests build and pass with
this change made.
2. KAboutLicense::Private ctors all need to be updated to set
`_versionRestriction` as well (basically anywhere that `_licenseKey` is set,
needs to also set `_versionRestriction`).
Once those fixes are made you have my +1, don't need to wait for a separate
review for it unless you want another look.
INLINE COMMENTS
> kaboutdata.cpp:166
> _licenseKey(Unknown),
> _aboutData(aboutData)
> {
Need a setter here for `_versionRestriction`
> kaboutdata.cpp:175
> _pathToLicenseTextFile(other._pathToLicenseTextFile),
> _aboutData(other._aboutData)
> {}
Need a setter here for `_versionRestriction`
> kaboutdata.cpp:206
> KAboutLicense::KAboutLicense(LicenseKey licenseType, const KAboutData
> *aboutData)
> - : d(new Private(licenseType, aboutData))
> + : KAboutLicense(licenseType, OrLaterVersions, aboutData)
> +{
We use `OnlyThisVersion` elsewhere as a default version restriction; using
`OrLaterVersions` here causes the auto test you added to break as well. ;)
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D6672
To: sitter, sebas, mpyne
Cc: #frameworks
