leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.
Apart from these couple of details, it looks pretty good :) (i'd say just fix
and commit, but one of them's a tiny bit larger than just a typo fix ;) )
INLINE COMMENTS
> engine.cpp:174
> if (!m_installation->readConfig(group)) {
> + Q_EMIT signalError(i18n("Failed to read config file %1",
> configfile));
> return false;
"Failed to read config file" is more a specific issue with setting up the
installation handler, so... "Could not initialise the installation handler for
%1. This is a critical error and should be reported to the application author"
would probably be a little clearer... It is /supposed/ to not happen, and is
really quite critical, so the best we can reasonably do is direct the blame to
the appropriate location.
> installation.h:78
> +#if KNEWSTUFFCORE_ENABLE_DEPRECATED_SINCE(5, 71)
> + KNEWSTUFFCORE_DEPRECATED_VERSION(5, 71, "No longer use, feature
> obsulete")
> bool isRemote() const;
Obsolete, not obsulete :) (just a nitpick, really :) )
REPOSITORY
R304 KNewStuff
REVISION DETAIL
https://phabricator.kde.org/D29455
To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
