kossebau added a comment.

  (Quick drive-by comment only as I had this in a search result...)

INLINE COMMENTS

> slavebase.h:351
>       */
> -    bool configValue(QString key, bool defaultValue) const;
> +    bool configValue(const QString &key, bool defaultValue) const;
>  

Would be a proper change, but sadly also changes the signature of a API under 
ABI promise, so here and in all other public API places not possibe.
Can be only done when breaking the ABI with KF6.

Should there be something else done instead? IMHO no, neither adding a TODO or 
even already an overload of the method with the preferred signature.
For one, checking and improving all API of KF6 with clazy should be expected to 
be done before any first release, so this should be catched at that time. At 
the same time, this change is source-compatible, client-side code does not have 
to be changed, so there is no real gain in client code readability, and the 
runtime price tag of a flat QString copy here is equally small compared to 
having another symbol and implementation code, so less code is better here.

See also 
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, dfaure
Cc: kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Reply via email to