Mir

Code review comment for lp:~andreas-pokorny/mir/add-dispatchable-alarm-factory

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> Overall: what is the purpose of this alarm factory? I presume it's to get key-
> repeat timers that are dispatched appropriately?

Yes. Not sending repeat events on a different thread than all other input events. Not having to pass *the* mainloop instance to the platform, where device tracking and handling repeat is even simpler. Oh and yes .. creating multiple mainloops also has fascinating effects that I want to avoid.

>
> Nit:
> + struct Alarm;
> + struct AlarmData;
>
> Both of these have significant interfaces and significant internal state; is
> there any reason why they're not declared as classes? Relatedly: C++ *really*
> should have not introduced the “class” keyword.

Of course they started out as simple structs before I remembered all the gory behavior details of Alarms..

> The code looks fine, although you should add documentation for the new public
> interfaces.

sure

« Back to merge proposal