KGuiAddons Review

[David Faure]

On Tuesday 04 March 2014 18:46:53 John Layt wrote:
> Hi,
> 
> Here's my first pass through KGuiAddons, focussing on the public api.
> 
> KColorCollection
> - Should probably become a QSharedDataPointer

Maybe. But OTOH it's just a QList, two QStrings and a an enum.

> KWorkdWrap
> - "// KDE5 TODO: return a value, not a pointer, and use QSharedDataPointer."

Done.

> KModifierKeyInfo
>  - Generally looks OK
>  - Has lots of "bool isKeyPressed(Qt::Key)" style methods and
> keyPressed(Qt::Key, bool) style signals when perhaps a KeyState enum would
> be better?

If it's just false/true it seems fine to me, the meaning is in the method name 
(so this is not the "API boolean trap").

>  - Uses X11 / XKB / XCB, will need Wayland backend eventually?

Yes, and Mac, and Windows.

>  - Perhaps really belongs in Qt?

Yes, but this requires providing implementations for all tier1 platforms in 
one go...

[snip]

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5