D6513: Add support for Attica tags support

David Faure noreply at phabricator.kde.org
Thu Sep 6 21:07:03 BST 2018


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> tagsfilterchecker.cpp:33
> +    {
> +        qDeleteAll(validators.values());
> +    }

Unnecessarily slow. Remove the .values() call.

> tagsfilterchecker.cpp:46
> +            m_tag = tag;
> +            if (value != QString::null) {
> +                m_acceptedValues << value;

!value.isEmpty()

Or isNull()?

> tagsfilterchecker.cpp:102
> +            if (!val) {
> +                val = new EqualityValidator(tag, QString::null);
> +                validators[tag] = val;

Consider QString::null deprecated. Use QString().

> tagsfilterchecker.cpp:103
> +                val = new EqualityValidator(tag, QString::null);
> +                validators[tag] = val;
> +            }

.insert(tag, val) is raster, see Effective STL

> tagsfilterchecker.cpp:113
> +                val = new InequalityValidator(tag, QString::null);
> +                validators[tag] = val;
> +            }

Insert

> CMakeLists.txt:4
>  
> -find_package(Qt5 ${REQUIRED_QT_VERSION} CONFIG REQUIRED Test Widgets) # Widgets for KMoreTools
> +find_package(Qt5 ${REQUIRED_QT_VERSION} CONFIG REQUIRED Test Widgets Gui Quick) # Widgets for KMoreTools and Qick for the interactive KNS test
> +

Qick ? :)

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, apol, #kde_store, whiting, ahiemstra, mlaurent, dfaure, cfeck
Cc: dfaure, cfeck, mlaurent, ngraham, ahiemstra, kde-frameworks-devel, #knewstuff, michaelh, ZrenBot, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180906/cc4b2e69/attachment.html>


More information about the Kde-frameworks-devel mailing list