leinir requested changes to this revision. leinir added a comment. This revision now requires changes to proceed.
Looks pretty good to me. It'd be nice if we could avoid exposing DocumentImpl (it's supposed to be sort of hidden), if you could have a think on a way to avoid that, i'd appreciate it :) INLINE COMMENTS > View.cpp:139 > + if (pageCache() == withCache) > + return; > + i'm always very uncomfortable with introducing returns inside statements... i'd very much like if you could refactor this so it doesn't just return like this (perhaps merge the two if statements, something like `if (pageCache() != withCache && d->document && d->document->impl())`?). i know it's semantically identical, but maintainability of early returns is just... super awkward ;) > KWCanvasBase.h:106 > virtual void setCacheEnabled(bool enabled, int cacheSize = 50, qreal > maxZoom = 2.0); > + bool isCacheEnabled() const; > It'd be nice to have docs on new public APIs, please :) i realise it's trivial and "just right there", but the api docs page doesn't show stuff quite so conveniently without a bit of help ;) REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15775 To: dcaliste, leinir, danders, anthonyfieroni, #calligra:_3.0 Cc: boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever
