D29455: KNS: Deprecate isRemote method and handle parse error properly
This revision was automatically updated to reflect the committed changes. Closed by commit R304:9575f2defb27: KNS: Deprecate isRemote method and handle parse error properly (authored by alex). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29455?vs=82098=82190 REVISION DETAIL https://phabricator.kde.org/D29455 AFFECTED FILES src/core/CMakeLists.txt src/core/engine.cpp src/core/installation.cpp src/core/installation.h To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29455: KNS: Deprecate isRemote method and handle parse error properly
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Sorted, nicely done :) Makes the code just a touch simpler as well, which is always good :) REPOSITORY R304 KNewStuff BRANCH fix_isremote_stuff (branched from master) REVISION DETAIL https://phabricator.kde.org/D29455 To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29455: KNS: Deprecate isRemote method and handle parse error properly
alex marked 2 inline comments as done. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D29455 To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29455: KNS: Deprecate isRemote method and handle parse error properly
alex updated this revision to Diff 82098. alex marked an inline comment as done. alex added a comment. Typo and error message If the error message is on two lines they aren't very readable, but that is an issue for another day :-). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29455?vs=82024=82098 BRANCH fix_isremote_stuff (branched from master) REVISION DETAIL https://phabricator.kde.org/D29455 AFFECTED FILES src/core/CMakeLists.txt src/core/engine.cpp src/core/installation.cpp src/core/installation.h To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D29455: KNS: Deprecate isRemote method and handle parse error properly
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
D29455: KNS: Deprecate isRemote method and handle parse error properly
alex updated this revision to Diff 82024. alex retitled this revision from "KNS: Remove isRemote method and handle parse error properly" to "KNS: Deprecate isRemote method and handle parse error properly". alex added a comment. Make isRemote deprecated REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29455?vs=82019=82024 BRANCH fix_isremote_stuff (branched from master) REVISION DETAIL https://phabricator.kde.org/D29455 AFFECTED FILES src/core/CMakeLists.txt src/core/engine.cpp src/core/installation.cpp src/core/installation.h To: alex, #knewstuff, ngraham, leinir Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
