Mir

Code review comment for lp:~vanvugt/mir/surface-states

Revision history for this message
Kevin DuBois (kdub) wrote :

> I don't see the utility of EventQueue as thing stand currently. I think a
> msh::Surface only needs an EventSink (i.e. an event handler). If we need an
> EventQueue in the future, it should be derived from EventSink. The surface
> doesn't need to know anything about the details of how its events are handled.
>
> Also, I would argue that since the id and the event handler are not really
> optional for the correct functioning of mir, nor there is any need to reset
> them, they should be taken at construction time:
>
> msh::Surface(..., id, event_sink, ...)
>
> msh::SurfaceFactory::create_surface(params, id/id_generator, EventSink)
that makes sense to me too.

1399 +TEST(EventQueue, events_get_handled)
we test that the same thing (but with different arguments) happens 3 times. We could test that one event gets handled and cover the same bit of code. (same sort of thing with ShellSurface.states)

The acceptance test names don't really capture what they're doing.
Its good to see a new acceptance test though :) I've been meaning to complain that the #of unit tests are expanding a lot faster than our integration/acceptance.

« Back to merge proposal