D8583: Add semantic data extraction plugin

Laurent Montel noreply at phabricator.kde.org
Wed Nov 1 06:45:05 GMT 2017


mlaurent added inline comments.

INLINE COMMENTS

> structureddataextractortest.cpp:41
> +            const auto refFile = dir.path() + QLatin1Char('/') + file.left(file.size() - 5) + QStringLiteral(".json");
> +            if (!QFile::exists(refFile))
> +                continue;

Perhaps signal that file doesn't exist here. For debug

> CMakeLists.txt:11
> +qt5_add_resources(semantic_srcs templates.qrc)
> +ecm_qt_declare_logging_category(semantic_srcs HEADER semantic_debug.h IDENTIFIER Log CATEGORY_NAME org.kde.pim.messageviewer.semantic)
> +

SEMANTIC_LOG is better that LOG :)

> datatypes.h:34
> +    Q_GADGET
> +    Q_PROPERTY(QString name MEMBER m_name)
> +    Q_PROPERTY(QString iataCode MEMBER m_iataCode)

Perhaps we can set CONSTANT no ?

> jsonlddocument.cpp:92
> +
> +QVariantList JsonLdDocument::fromJson(const QJsonArray& array)
> +{

coding style space before & not after

> semanticmemento.h:36
> +    // TODO
> +    QJsonArray data;
> +};

mData ? as variable of class

> structureddataextractor.cpp:31
> +
> +void StructuredDataExtractor::parse(const QString& text)
> +{

coding style "QString &"

> structureddataextractor.cpp:34
> +    parseXml(text);
> +    if (m_data.isEmpty())
> +        findLdJson(text);

if (m_data.isEmpty()) {
      findLdJson(...);
      if (m_data.isEmpty()) {
          parse....
      }
  }

> structureddataextractor.cpp:40
> +
> +void StructuredDataExtractor::parseXml(const QString& text)
> +{

coding style QString &

> structureddataextractor.cpp:160
> +    QString v;
> +    if (reader.name() == QLatin1String("meta"))
> +        v = reader.attributes().value(QLatin1String("content")).toString();

cache reader.name() as QStringRef readerName = reader.name();

REPOSITORY
  R81 KDE PIM Addons

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

To: vkrause
Cc: mlaurent, #kde_pim, dvasin, winterz, vkrause, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20171101/bed121a2/attachment.html>


More information about the kde-pim mailing list