Mir

Code review comment for lp:~robertcarr/mir/receive-input-in-client

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

>So, we can probably land this, and do some followup cleaning:

+1

...
> src/client/input/android_input_platform.{cpp,h} is GPlv3+; should be LGPLv3+
> src/client/input/android_input_receiver.{cpp,h} is GPlv3+; should be LGPLv3+
> src/client/input/android_input_receiver_thread.{cpp,h} is GPlv3+; should be
> LGPLv3+
> src/client/input/input_platform.h is GPlv3+; should be LGPLv3+
> …and basically all the added files, it seems.
>
> A bunch of cases of ~Foo() {}, which I think would more idiomatically be
> ~Foo() = default;

We can probably fix this when we clean up a whole bunch of potentially throwing destructors.

> 2641 +// thread->join();
>
> Looks like you've got some temporary testing code still hanging around?
>
> 2883 - auto surface = std::make_shared<MirSurface> (connection.get(),
> 2884 -
> *client_comm_channel,
> 2885 - logger,
> 2886 - mock_buffer_factory,
> 2887 - params,
> 2888 - &empty_callback,
> 2889 - nullptr);
> 2890 + auto surface = std::make_shared<MirSurface> (connection.get(),
> *client_comm_channel, logger, mock_buffer_factory, input_platform, params,
> nullptr, &empty_callback, nullptr);
> (and further examples in that file)
> I think we prefer the former formatting, with shorter lines?

I think we should prefer a third style - breaking before the first parameter when necessary. (The original is fragile when renaming anything that appears before that point - and we do global renames often enough for it to matter.)

review: Approve

« Back to merge proposal