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

[Dan Leinir Turthra Jensen]

leinir marked 3 inline comments as done.
leinir added inline comments.

INLINE COMMENTS

> broulik wrote in author.h:111
> `ProfilePage`?

In principle yes, except we can't change homepage to homePage, until KF6... and then we'd end up with mixed spelling styles, and that just makes me sad ;)

> broulik wrote in commentsmodel.cpp:184
> Any particular reason not do just return a null `QVariant()`?

Hmm... This actually is supposed to be "Unknown model role"... It's kind of corner-casey, but i've noticed in the past that something other than just an invalid QVariant is occasionally useful for those times where you've, say, missed the h in "depth" ;) Usually this wouldn't be hit, of course, so it's not actually expensive in any real way. But yes, compared to this, QVariant() would be the better option.

> broulik wrote in commentsmodel.h:70
> Why is this an explicit `Engine` pointer rather than generic `QObject`?

Entirely to make it awkward for people to use it in a counter-intended fashion :)

> broulik wrote in commentsmodel.h:75
> You might want to set that to `Qt::DisplayRole`

Hmm... i guess, except that it's not reeeally the obvious choice (because i don't really see one of those)... but yeah, having one is probably not terrible anyway.

> broulik wrote in commentsmodel.h:88
> `QModelIndex` is usable from QML these days, this overload isn't neccessary, you can do from QML:
> 
>   model.data(model.index(row, column), role);

Aah! i did not know this, very handy indeed. Couple of places where that'll make life simpler :)

> broulik wrote in engine.cpp:803
> ah, so this is why it's not `const`

Indeed :) (and yes, that cache still needs to be made...)

REPOSITORY
  R304 KNewStuff

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

To: leinir, #knewstuff, #vdg, #frameworks, ahiemstra
Cc: davidedmundson, 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/20190909/8e977fc0/attachment-0001.html>