D21721: Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> author.h:111
> +     */
> +    void setProfilepage(const QString &profilepage);
> +

`ProfilePage`?

> commentsmodel.cpp:184
> +            default:
> +                value.setValue(i18nc("The value returned for an unknown role when requesting data from the model.", ""));
> +                break;

Any particular reason not do just return a null `QVariant()`?

> commentsmodel.h:70
> +     */
> +    explicit CommentsModel(Engine *parent = nullptr);
> +    virtual ~CommentsModel();

Why is this an explicit `Engine` pointer rather than generic `QObject`?

> commentsmodel.h:71
> +    explicit CommentsModel(Engine *parent = nullptr);
> +    virtual ~CommentsModel();
> +

`~CommentsModel() override;`?

> commentsmodel.h:75
> +        IdRole = Qt::UserRole + 1,
> +        SubjectRole,
> +        TextRole,

You might want to set that to `Qt::DisplayRole`

> commentsmodel.h:88
> +    QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
> +    Q_INVOKABLE QVariant data(int index, int role  = Qt::DisplayRole) const;
> +    int rowCount(const QModelIndex& parent = QModelIndex()) const override;

`QModelIndex` is usable from QML these days, this overload isn't neccessary, you can do from QML:

  model.data(model.index(row, column), role);

> engine.cpp:803
> +    model->setEntry(entry);
> +    // Cache the models mebby?
> +    // Track onDestroyed and remove from cache...

ah, so this is why it's not `const`

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra
Cc: broulik, ahiemstra, anthonyfieroni, pino, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190908/86ae0b47/attachment-0001.html>