D23523: [SlaveBase] Use QMap instead of KConfig to store ioslave config

David Faure noreply at phabricator.kde.org
Mon Sep 2 13:25:40 BST 2019


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


  I admit that I never understood why this was using KConfig. I guess Waldo had a hammer and everything looked like nails ;-)

INLINE COMMENTS

> slavebase.cpp:191
> +        delete configGroup;
> +        delete config;
>      }

missing `config = nullptr` so the `delete config` in the destructor doesn't crash.

Same with configGroup.

> slavebase.cpp:411
>  
> +QMap<QString, QString> &SlaveBase::mapConfig() const
> +{

That is a weird abuse of const. If the member wasn't in a d pointer, this wouldn't compile.

If the slave is supposed to only read from the map, then it should be a value or const ref.
If the slave can write into the map, then this method shouldn't conceptually be `const`.

> slavebase.h:342
> +     * relevant for the current protocol and host.
> +     */
> +    QMap<QString, QString> &mapConfig() const;

missing @since

> slavebase.h:353
> +     * TODO KF6: remove
> +     * @deprecated use mapConfig(StatSide side)
>       */

@deprecated since 5.xx

REPOSITORY
  R241 KIO

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

To: meven, davidedmundson, dfaure, #frameworks
Cc: broulik, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190902/b53d8156/attachment.html>


More information about the Kde-frameworks-devel mailing list