D12616: Akregator feed detector plugin: Port away from KDELibs4Support

[David Faure]

dfaure accepted this revision as: dfaure.
dfaure added a comment.
This revision is now accepted and ready to land.


  Found stuff on the way, but the commit itself is fine.

INLINE COMMENTS

> CMakeLists.txt:15
>  
> +ecm_qt_declare_logging_category(akregatorplugin_DEBUG_SRCS HEADER akregatorplugindebug.h IDENTIFIER AKREGATORPLUGIN_LOG CATEGORY_NAME org.kde.konqueror.akregatorplugin)
> +

Installing a categories file as well (like e.g. kbookmarks/kbookmarks.categories, or many others) would make that logging category available in kdebugsettings.

> pluginbase.cpp:35
> +#include <qurl.h>
>  #include <QtDBus>
> +

(pre-existing) this includes all of QtCore as well... better use more specific includes

> pluginbase.cpp:77
>          if (s2.startsWith(QLatin1String("//"))) {
> -            s2 = s2.prepend(baseurl.protocol() + ':');
> -            u = s2;
> +            s2 = s2.prepend(baseurl.scheme() + ':');
> +            u.setUrl(s2);

(pre-existing) prepend modifies the string, so "s2 = " is redundant

> pluginbase.cpp:81
> +            QUrl b2(baseurl);
>              b2.setPath(QString()); // delete path and query, so that only protocol://host remains
>              b2.setQuery(QString());

(pre-existing) technically a fragment could remain too, here....

> pluginbase.cpp:92
> +    u = u.adjusted(QUrl::NormalizePathSegments);
> +    //qCDebug(AKREGATORPLUGIN_LOG) << "url=" << s << " baseurl=" << baseurl.url() << "fixed=" << u.url();
>      return u.url();

(pre-existing) the two .url() calls should be removed. This is especially important in case a password ends up in the URL (we don't want it printed out in the debug output).

> pluginbase.h:46
>       */
>      bool akregatorRunning();
>      /**

(pre-existing) could be static

REPOSITORY
  R226 Konqueror

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

To: marten, #konqueror, #plasma, dfaure
Cc: dfaure, apol, anthonyfieroni
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180511/96bc1bcc/attachment.htm>