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
