D6672: add KAboutLicense::spdx and introduce orLater qualification

Michael Pyne noreply at phabricator.kde.org
Thu Jul 13 16:25:02 UTC 2017


mpyne requested changes to this revision.
mpyne added a comment.
This revision now requires changes to proceed.


  AFAICT we do need to maintain BIC here even for private ctors.  The inline comment has more detail.  Other than that and with using a flag enum instead of a `bool` I like the patch and the approach, and how you've maintained the same basic approach the existing code does.
  
  I would lean towards also providing the convenience accessor in `KAboutLicense` for the orLater flag, but it is already exposed back to the user so perhaps it wouldn't add enough extra detail to be worth it.

INLINE COMMENTS

> kaboutdata.h:291
> +    explicit KAboutLicense(enum KAboutLicense::LicenseKey licenseType, const KAboutData *aboutData,
> +                           bool orLater = false);
>      /**

Regarding the BIC question, we do consider this BIC, even though it is SC thanks to the default param.  Instead a second ctor is needed with this signature (and w/out the default param to avoid C++ errors), with a comment to merge with the existing ctor in KF6.

Please see https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B, you can search for "extending a function with another parameter" to see what I'm referring to.  This guidance applies to methods of any type, even private methods.

However, I think the `orLater` param would be more understandable as an enum flag, in the same way we modified our APIs in KDE4 and KF5 to try and avoid `bool` params.  This would be especially important for readability if we do start to support things like license exceptions, you can imagine future calls would then look like `...->addLicense(KAboutLicense::LGPL_V3, true, KAboutLicense::ExceptionGeneratedCodeUse);` and then wonder what the `true` was for.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D6672

To: sitter, sebas, mpyne
Cc: #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170713/aba49209/attachment.html>


More information about the Kde-frameworks-devel mailing list