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
