[PATCH weston 00/16] IVI Layout Notification APIs Rework

[Ucan, Emre (ADITG/SW1)]

Hi Pekka,

Yes, it makes sense to squash patches. I will also add the missing documentation about the listener APIs.
I will send modified patches soon. Thank you for your review.

> -----Original Message-----
> From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
> Sent: Freitag, 1. April 2016 13:44
> To: Ucan, Emre (ADITG/SW1)
> Cc: wayland-devel at lists.freedesktop.org; Natsume, Wataru (ADITJ/SWG)
> Subject: Re: [PATCH weston 00/16] IVI Layout Notification APIs Rework
> 
> On Thu, 31 Mar 2016 11:08:46 +0000
> "Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:
> 
> > I changed the interface of the notification APIs to get a wl_listener
> > instead of a ivi-shell specific notification callback function.
> >
> > This change has several advantages:
> > 1. Code cleanup
> > 2. No dynamic memory allocation. Listeners are allocated
> >    by controller plugins
> > 3. Remove API is not needed. Controller plugins can easily
> >    remove the listener link.
> >
> > The first patch is a preparation for the notification API changes.
> > It moves the event_mask to ivi_layout_surface/layer_properties struct,
> > so that it is accessible by get_properties_of_layer/surface APIs.
> > Then, it is not needed to send event_mask with an ivi-shell callback
> function.
> >
> > The patches 2-15 modifies and renames IVI Layout notification APIs,
> > and removes their remove APIs.
> >
> > The 16th patch removes content_observer callback function which is a
> > leftover of the patch: 193e301c740b582f20307e3e08f8373d26ea968f
> >
> > Emre Ucan (16):
> >   ivi-shell: move event_mask to properties struct
> >   ivi-shell: rework surface_add_notification API
> >   ivi-shell: remove surface_remove_notification APIs
> >   ivi-shell: rework layer_add_notification API
> >   ivi-shell: remove layer_remove_notification APIs
> >   ivi-shell: rework create_surface notification
> >   ivi-shell: remove remove_notification_create_surface
> >   ivi-shell: rework create_layer_notification
> >   ivi-shell: remove remove_notification_create_layer
> >   ivi-shell: rework remove_layer notification
> >   ivi-shell: remove remove_notification_remove_layer
> >   ivi-shell: rework remove_surface notification
> >   ivi-shell: remove remove_notification_remove_surface
> >   ivi-shell: rework configure_surface notification
> >   ivi-shell: remove remove_notification_configure_surface
> >   ivi-shell: remove content_observer leftover
> >
> >  ivi-shell/hmi-controller.c       |   42 +--
> >  ivi-shell/ivi-layout-export.h    |  113 +-------
> >  ivi-shell/ivi-layout-private.h   |    2 -
> >  ivi-shell/ivi-layout.c           |  545 +++++---------------------------------
> >  tests/ivi_layout-internal-test.c |   76 +++---
> >  tests/ivi_layout-test-plugin.c   |   93 ++++---
> >  6 files changed, 209 insertions(+), 662 deletions(-)
> >
> 
> Hi,
> 
> I had separate comments on patch 2, which probably needs some bits of
> patch 3.
> 
> Somehow I would assume that patch 4 would have a similar issue with
> ivi_layout_layer_remove_notification() call being removed only in patch 5,
> but it didn't trigger for me.
> 
> Considering the add and remove notifier/listener are two sides of the same
> thing, maybe it would be logical to do both in the same patch?
> That is, squash 2 and 3, 4 and 5, etc.?
> 
> For all the add_listener public API functions you should document what the
> void *data argument to the wl_listener::notify callback is.
> 
> In the great assignment block of static struct ivi_layout_interface
> ivi_layout_interface it is ok to keep a comma also after the last item.
> It helps keeping diffs one change smaller.
> 
> It is not needed to add a cast to/from void*.
> 
> Patches 1 and 16 are R-b me and pushed:
>    87953b7..0bd29b6  master -> master
> 
> Patches 2 - 15 I hope to see revised.
> 
> 
> Thanks,
> pq

Best regards

Emre Ucan
Software Group I (ADITG/SW1)

Tel. +49 5121 49 6937